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

Check your claim page changes #2340

Merged
merged 53 commits into from
May 14, 2018
Merged

Check your claim page changes #2340

merged 53 commits into from
May 14, 2018

Conversation

lostie
Copy link
Contributor

@lostie lostie commented Apr 23, 2018

What

  • Re-styling the "Check your claim" page and add links to each of the sections so users can edit them directly, rather than having to go step by step until reaching the named section.
  • When the users are on the claims list page and click on a claim that is in draft the page shown is the "Check your claim" page
  • The users are not able to save as draft for case details and transfer fee details as their the most basic information that is required in order for the claim to exist in the system. NOTE: This only applies for the web submissions. API submissions remained the same.
  • In the web submissions, each form step is now validated independently from any other steps. NOTE: This only applies for the web submissions. API submissions remained the same.
  • DRY form templates. Remove repetition from form templates (first step for this process, that requires further DRYness).
  • "Check Your Claim" page display a top level banner if the claim is NOT valid and informs the user of which sections still need to be completed.
  • For each section, if the section cannot be edited, an info banner is displayed informing the user of which other sections need to be completed before being able to edited.
  • Assigns total attributes for litigator final/interim/transfer claims before validation so that its possible to correctly validate the associated fees total in each of their own steps.

Ticket

Change links on the 'check your claim page'

@lostie lostie added the WIP label Apr 23, 2018
@lostie lostie force-pushed the check-your-claim-page-changes branch from 3ce7cf2 to f0a47e9 Compare April 25, 2018 16:04
@lostie lostie requested a review from cleargif April 26, 2018 12:48
@lostie lostie force-pushed the check-your-claim-page-changes branch 5 times, most recently from 329c161 to 96cbaff Compare May 1, 2018 13:17
Sergio Marques added 17 commits May 2, 2018 09:54
All the check your page claim was doing was a whole bunch of if/else
statements to display sections depending on what type of claim and what
kind of information it has.

These changes do 2 things:
- the sections that are required for a given type of claim are now
defined in its associated presenter and the view just iterates over
those sections.
- if a given section is required for a type of claim but its not to be
displayed given some data the claim has, that decision is taken on the
template for that section.
The re-styling applies for all kind of fees that make use of the
summary_fees template (e.g. fixed fees, basic fees, etc)
page

Also: Re-styling of the warrant fee page
- Validate only the fields for the current step
- Preserve existent validation behaviour for the API submissions
has_one/has_many validations for web submissions should only be
validated if they're associated with the current step.
With the introducion of editing a claim step directly, there will be
cases where the fee trying to be displayed in the summary is not yet
filled. For those cases, the template for the fee summary should not be
rendered as there's no info to display.
@lostie lostie force-pushed the check-your-claim-page-changes branch from 96cbaff to 43a48c1 Compare May 2, 2018 09:03
@@ -25,15 +25,18 @@ def type_identifier
'lgfs_interim'
end

def raw_fixed_fees_total
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This doesn't seem to be needed for this type of claim.

Sergio Marques added 3 commits May 2, 2018 11:47
- If the claim is in draft the edit link opens the "Check your claims
page"
- If the claim in NOT in draft the edit link opens the "Summary page"
Was left from a previous commit.
@lostie lostie requested a review from colinbruce May 8, 2018 11:57
@lostie lostie force-pushed the check-your-claim-page-changes branch from e129c17 to 3d89dec Compare May 8, 2018 12:54
Sergio Marques added 5 commits May 8, 2018 13:55
The claim page will show 2 new components:
- A top level component that applies to the claim, which will be
displayed if the claim is not valid to be certified.
- A component per section, which will be displayed if that given section
requires some other sections to be filled before it can be completed.

Also:
- Refactored the logic to determine the validity of a claim/section.
Moved that logic to services given there was too much complexity to be
added directly to the claims model.

TODO: There's still quite a lot of DRY work to do, but these changes are
becoming to big to be able to address them:
- DRY summary pages. There's a lot of repetition in the summary page,
specially for the headers/status of the page
(CYC - Check your claim)

When a type of fees is being displayed in the CYC page and there's no
fees for it, the page displays the appropriate wording instead of an
empty list.
Both new/create actions for the certifications controller were changed
to have a before hook to assert that the claim is valid and can be
certified.

If the claim is not valid, the user is redirected to the "Check Your
Claim" page and an error message is displayed.

Also:
- Removed duplicate check on new action for status of claim (was already
being done in a before hook)
- The new action used to validate the claim based on the current step
(at least there's step being step). With the changes introduced to the
validations to have them perform in isolation for each step, the
certifications would validate ONLY the default step (case details for
most of the claims). These changes guarantee that the validation is
performed on all the steps of the claim, which should be the only way to
proceed to the certification.
Due to the existent validation behaviour some fields, like total were
being checked at the very end of the submission.

After the changes to validate ONLY each step as required, these field
validations need to be performed in the appropriate step where they're
actually required.
Copy link
Contributor

@colinbruce colinbruce left a comment

Choose a reason for hiding this comment

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

Looking good...

Manual testing of API testing starts now

summary: 'Note: To best navigate this table, select the right hand column and then step down through the rows.'
tranfer_detail:
transfer_detail:
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Sergio Marques added 8 commits May 8, 2018 17:07
What:
- Case details are no longer saved as draft as are considered mandatory
information. The save as draft functionality is tested on the defendants
page instead
The totals (fees, expenses, vat, etc) are now validated in the
appropriate steps where they're required.
Because in the first instance
the fees are not yet persisted during the validations, a 'total amount
claimed must be greater than 0' was being displayed. The reason for that
is the totals were only being updated after the records were saved.

These changes, ensure for non-API submissions (still need to check if
the API should follow the same behaviour or not) the totals are set
before validation so we can assert they're valid as expected.

Also:
- Refactored the calculations module to allow collections or single
records of fees to be provided for the totals calculation.
CYC - Check Your Claim page

- If there’s content for the section, the missing information message
should not appear
- If there’s no content for the section, only the missing information
message should appear
Given the user can only submit the transfer details without case
details, the "Check Your Claim" page should show the other sections
cannot be filled in without the case details.
@lostie lostie force-pushed the check-your-claim-page-changes branch 2 times, most recently from 8783912 to 779c0bb Compare May 10, 2018 08:48
Sergio Marques added 3 commits May 10, 2018 12:24
Was a URL and was not safe to just pass whatever we received.

Now it checks for the referrer param to be passed, and if it is, it maps
it into a referrer URL if it matches any valid whitelist referrer.
CYC - Check Your Claim page

The rationale behind this change has to do with the fact that these type
of fees are currently created blank before hand. To keep this behaviour
without impacting the rest of the application, the CYC page does not
display any basic fees if they're all blank.
Until now the validation failed even when there wasn't fees set.

That was because the check was being made as:

total_defendant_uplifts_fees (0) + 1 > number_of_defendants (0)

Going forward the validation is performed only when there's at least a
fee set.
@lostie lostie force-pushed the check-your-claim-page-changes branch from 779c0bb to 4563725 Compare May 10, 2018 11:24
Sergio Marques added 3 commits May 10, 2018 13:41
Given the current submitted data, if a section cannot be accessed (e.g.
Travel expenses for LGFS Interim claims should only be accessed if a
warrant fee is set), that section should not be displayed.
If not the "Check Your Claim" page will display a message indicating
they haven't instead of the list with empty fields.
So that we can decide if the section is displayed or not in the "Check
Your Claim" page
@lostie lostie removed the WIP label May 10, 2018
@lostie lostie merged commit 5f969e0 into master May 14, 2018
@lostie lostie deleted the check-your-claim-page-changes branch May 14, 2018 11:07
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.

2 participants