Something seems wrong with CI. But I got no time to investigate right now..

The tests for amivapi/tests/studydocs/test_studydocs_auth.py and amivapi/tests/studydocs/test_summary.py are failing.

Ah, because type is now required, and of course the old tests do not specify the type.

If anybody has time, this should be an easy fix. I'll do it as soon as I can, but not sure when that will be :/

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (2cbe03f) 94.75% compared to head (8bd9cdb) 94.77%.

@@            Coverage Diff             @@
##           master     #423      +/-   ##
==========================================
+ Coverage   94.75%   94.77%   +0.02%     
==========================================
  Files          75       75              
  Lines        4329     4329              
==========================================
+ Hits         4102     4103       +1     
+ Misses        227      226       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@NotSpecial The change of the type options for study documents do not come with any data migrations, so the existing study documents will still come with the type exams. This case is not covered by the tests as the API does not accept an item with type exams anymore.

Do you have any idea how the API handles this situation?

I just saw the use of self.load_fixture(...) in other tests. We could add some additional tests to cover that scenario by adding the entities with load_fixture instead of the regular POST endpoint of the API.

The API will enforce the new type for all new requests. Existing data will be returned as-is. We would need to migrate it ourselves.

Migration of existing data is difficult because we can only partially automate the process by looking up lectures in the course catalogue of ETH and match those lectures which still exist with the same name.
If we keep the old entries as is, we might need to have additional filter options in addition to the document types officially supported by the API in order to be able to find all documents by filtering. This is also not ideal.

I'm not sure what to tell you 😅

Data migration is always a difficult task, that's why changes to the data schema are only done when really necessary.

So considering that, is the split into autumn/spring exams really that necessary?
We could easily revert this part of the PR and go back to just exam.

I was just implementing the suggestion in #422, but now that I see it again I am not sure whether it's really that helpful to split the exam types up.

What's your opinion? Should we just keep exam as before? I mean usually, the exam is in the same semester as the lecture, so looking for lectures in HS with an exam is the same as looking for lectures with an autumn exam.

I think that having the document type exam would be sufficient.
It can happen that a lecture gets moved from the autumn semester to the spring semester or vice versa. When this happens, I would want to be able to get all exams for that lecture from the previous years nonetheless. So in this case, splitting the type would rather introduce more confusion.

I will check with the current board members how we should proceed and then get back to you.

I haven't talked to the rest of the Board, but I think the separation into spring/ autumn exams is more hindering than helpful, as if I'm looking for old exams, I also want to find the ones in the off-season.

Additionally, people often mix things up. In the past, I downloaded old exams with e.g. 'spring20' in the doc-title, but the exam date was February 20 and should therefore have been 'autumn19'.

In that case, we should probably revert this part of the change and go back to a single exam type.

Please note that we might still have existing documents where this information is not set, so these documents should be migrated after this change has been deployed.

overall I like the idea. But I'm concerned about migration. also, because we will be migrating to community solutions in the future it might not make sense to do a lot of work on the migration, if that requires a lot of work.

The existing documents must be migrated to the new platform, so if there is more structured data available to perform this migration as automated as possible the better it is.
This change only applies to newly uploaded or edited documents, but those documents are less likely to require manual review for migration. So I think this a change like this still makes sense to complete especially in this case where the work has already been done.

The only change which might also be necessary is that the website form has to also enforce those fields for better feedback to the user.

ok if there is no need to migrate old documents then I'm happy with this PR.