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

#51 - Show errors to asssesors on late IAs #58

Merged
merged 8 commits into from Aug 7, 2020

Conversation

crablab
Copy link
Contributor

@crablab crablab commented Jul 31, 2020

This adds a number of things:

  • the Warning component from admin
  • the date helpers from admin
  • On /assessments an "overdue" call out when the assessment is past the deadline and incomplete
  • On /assessments a new disabled button for when the assessment is past the hard limit (and thus expired)
  • On /edit an "overdue" call out the Warning component when the assessment is past the deadline and incomplete
  • On /edit the Warning component when the assessment is past the hard limit and expired, with contact details for the teams

This currently won't do anything, as assessment.hardLimit isn't being copied across in the function. We may want to tweak some stuff when we see how this is all working together.

Overdue but within hard limit:
screenshot_2020-08-04-102551

Overdue and past hard limit:
screenshot_2020-08-04-102642

@warrensearle
Copy link
Member

Hey @crablab some of these changes aren't in the ticket #51 you're working on. Is there another ticket requesting the additional changes? Some of them reverse changes that were applied in the past - e.g. in #9 we removed the 'overdue' labels.

@crablab
Copy link
Contributor Author

crablab commented Jul 31, 2020

@warrensearle I think that's a bit of a strict interpretation to be honest. I thought these would be useful added value changes, in line with the overall aim of the ticket: to make it clear when an IA is running late and has expired.

@YaaL
Copy link
Contributor

YaaL commented Jul 31, 2020

@warrensearle with the soft and hard return date it makes sense to have the overdue label.
TBH I'm not sure why it was removed?

@warrensearle
Copy link
Member

@ClaireTroughtonJAC or @Rebecca-mcknight1991 please can you confirm the reasoning for removing the Overdue label. Suggest we move this discussion to slack / stand-up.

@ClaireTroughtonJAC
Copy link

@warrensearle The reason I asked for it to be removed is I don't think Overdue is not the best label for an assessor to see. I think the due date is sufficient. I think we inform Assessors of this in other ways of outstanding assessor.

@crablab
Copy link
Contributor Author

crablab commented Aug 3, 2020

@ClaireTroughtonJAC Is it best to not have the 'overdue' message at all, or is there better copy we could use? Just conscious that at the moment there is no indication on the IA pages that an assessment is late, and this could be a poor user experience if the assessor didn't realise and then exceeded the hard limit.

@ClaireTroughtonJAC
Copy link

Will review warning component later when on staging.

@crablab
Copy link
Contributor Author

crablab commented Aug 4, 2020

Discussed with Claire this morning and we're making the following changes:

  • Remove OVERDUE labels
  • Use warning component on /edit if an IA is running late

Potentially another ticket to use the warning component on /assessments.

@crablab
Copy link
Contributor Author

crablab commented Aug 7, 2020

@warrensearle would it be possible to get a review on this please?

@warrensearle
Copy link
Member

@crablab this all looks good. It's currently blocked by jac-uk/digital-platform#334 which is now in progress

@crablab
Copy link
Contributor Author

crablab commented Aug 7, 2020

@warrensearle Yup! As noted above, I think once that ticket is done we can test this and PR any required changes 😄

I'll merge this now then 👍

@crablab crablab merged commit e14533b into develop Aug 7, 2020
@crablab crablab deleted the feature/51-late-ia-error branch August 7, 2020 11:48
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.

None yet

4 participants