kit-sdq / artemis4j

REST Client for the Artemis Project
Eclipse Public License 2.0
5 stars 6 forks source link

Fix `getSubmissions` not returning all submissions #35

Closed Luro02 closed 10 months ago

Luro02 commented 10 months ago

Artemis allows non-instructors to fetch all submissions and not only the ones assessed by the tutor (with this change tutors will be able to use https://github.com/kit-sdq/programming-lecture-artemis-score-stats).

The original python API had this method signature for the endpoint:

    async def get_submissions(
        self,
        exercise_id: int,
        filter_submitted_only: bool = False,
        filter_assessed_by_tutor: bool = False,
        correction_round: int = 0,
    ) -> AsyncGenerator[ProgrammingSubmission, None]:

which allowed the caller to specify the options for the endpoint.

I tried to replicate the same behaviour for the Java API. The Javadoc of getSubmissions does not gurantee that the returned submissions are only those assessed by a tutor:

    /**
     * Get all submissions for the given exercise and correction round.
     *
     * @return submissions for the given exercise and correction round.
     * @throws ArtemisClientException if some errors occur while parsing the result.
     */

Therefore I changed the default to false, so it really returns "all submissions".

dfuchss commented 10 months ago

Thx for the PR!

Could you adapt the PR that it is backwards compatible. Especially, for the eclipse plugin, the plugin should show the assessed tasks in the backlog and not all.

Luro02 commented 10 months ago

Could you adapt the PR that it is backwards compatible.

How should I do this? Even if I change the boolean to true, the PR will still be backwards incompatible, because I changed the method in the interface.

dfuchss commented 10 months ago

You create a new method List<Submission> getSubmissions(Exercise exercise, int correctionRound, boolean filterAssessedByTutor). The old method (without bool) should behave same as before. Afterwards, we can use the new interface in the stats tool

Luro02 commented 10 months ago

@dfuchss fixed it

dfuchss commented 10 months ago

Ok .. I'll test it. Credentials are locked :)

dfuchss commented 10 months ago

CI is green :)