Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/embargo #521

Merged
merged 24 commits into from
Aug 9, 2023
Merged

Feature/embargo #521

merged 24 commits into from
Aug 9, 2023

Conversation

EdoStorm96
Copy link
Contributor

This PR introduces an embargo option for researchers to keep their applications out of the users only archive until a specified date. It also reorganizes the archive naming scheme in the code, adds a querysetManager for different Queryset needs, and reintroduced functionality for the secretary to manually change the archive status of a proposal.

Also contains a fix for issue 505

…earlier, more complicated, implementation commented out, which I will probably remove next commit...
'study_set__observation', 'study_set__session_set',
'study_set__intervention', 'study_set__session_set__task_set'
)
#breakpoint()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commented out breakpoint probably should be removed :)

Comment on lines +637 to +646
if cleaned_data['embargo'] is None:
self.add_error('embargo', _('Dit veld is verplicht.'))

embargo_end_date = cleaned_data['embargo_end_date']
two_years_from_now = timezone.now().date() + timezone.timedelta(days=730)

if embargo_end_date is not None and \
embargo_end_date > two_years_from_now:
self.add_error('embargo_end_date', _(
'De embargo-periode kan maximaal 2 jaar zijn. Kies een datum binnen 2 jaar van vandaag.'))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These checks should be added to the docstring

class ProposalQuerySet(models.QuerySet):

DECISION_MADE = 55
TODAY = datetime.date.today()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you define a class property and set it with an expression (like TODAY here), said expression is evaluated only once, namely when the app starts up.

Thus, TODAY will actually be THE_DAY_THE_APP_STARTED.

Seeing as the line is not that complicated, it's probably best to just call the method inside the methods in the class. To shorten it a bit, you could change the import to import date from datetime, and use date.today().

Comment on lines 308 to 311
_('Als de deelnemers van je onderzoek moeten worden misleid, kan \
je ervoor kiezen je applicatie pas later op te laten nemen in het \
semi-publieke archief. Wil je dat jouw onderzoek tijdelijk onder \
embargo wordt geplaatst?'),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The help text is misleading. It probably should clarify it will be hidden from both the public as well as the users only archive

@@ -33,13 +33,13 @@
path('export/', ProposalsExportView.as_view(), name='archive_export'),
path('export/<int:pk>/', ProposalsExportView.as_view(),
name='archive_export'),
path('hide/<int:pk>/', HideFromArchiveView.as_view(),
path('hide/<int:pk>/', ChangeArchiveStatusView.as_view(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very minor point, but the URL itself probably should be changed as well :)

@EdoStorm96 EdoStorm96 requested a review from tymees August 9, 2023 08:59
Copy link
Member

@tymees tymees left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should run makemessages once more, as you changed that help text.

Otherwise, looks good!

@EdoStorm96 EdoStorm96 merged commit e42a841 into develop Aug 9, 2023
1 check passed
@EdoStorm96 EdoStorm96 deleted the feature/embargo branch August 9, 2023 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants