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

Revision date in list & edit intended start date #570

Merged
merged 8 commits into from
Nov 8, 2023

Conversation

EdoStorm96
Copy link
Contributor

Fixes for issues #512 and #483. Just some niceties for Desiree. I'd say these work fine :)

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.

I didn't test it locally, but I don't see anything would warrant a local test assuming you did test your code ;)

2 textual changes I'd like to see. The other two comments are niceties, I'm fine with as they are now if you don't think the niceties are required

{% blocktrans trimmed with title=proposal.title ref_number=proposal.reference_number %}
Op deze pagina kan de startdatum worden aangepast van de aanvraag {{ title }}
(referentienummer <em>{{ ref_number }}</em>). <b>Let op!</b> Als de review al is afgerond,
wordt de nieuwe startdatum niet automatisch weergegeven in de PDF. Mocht u de PDF
Copy link
Member

Choose a reason for hiding this comment

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

'U' should be 'je'

Comment on lines 25 to 27
<p>
{% trans "Vul de datum in als 'DD-MM-YYYY'." %}
</p>
Copy link
Member

Choose a reason for hiding this comment

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

This is incorrect.

It might work, it will probably work when the locale is set to Dutch, but it might not always.

YYYY-MM-DD will always work

Or... Just use the custom year widget?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added the DateField widget from cdh core

Comment on lines 121 to 123
{# For some reason context.review.REVISION and other class variables of continuation are None?#}
{# Only context.review.GO returns 0 for me, as it should #}
{# Which is why I've hardcoded the 1 here ... #}
Copy link
Member

Choose a reason for hiding this comment

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

I can answer this one for you. This is a Vue template (and a first-generation one, which is a bit crap compared to what we're migrating to).

Thus, the context used here is not the same as what you'd expect. It's a manually created dict in reviews/api/views.py.
GO is in the dict; REVISION isn't. If you add it to the dict, you can reference it here.

Comment on lines 226 to 228
if is_secretary(user):
pdf_container.date_start_edit_link = reverse('proposals:update_date_start',
args=[proposal.pk])
Copy link
Member

Choose a reason for hiding this comment

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

I don't really like adding this link to the document list code. You're not updating documents.

This should be a review action instead, I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that does make more sense! fixed

@EdoStorm96 EdoStorm96 merged commit 84df961 into develop Nov 8, 2023
2 checks passed
@EdoStorm96 EdoStorm96 deleted the feature/revision_date_in_list branch November 21, 2023 14:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Date sent for revision on 'In revision' page? DMP and application editing possibilities secretary
2 participants