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

EPP-146 amend CHANGE SUBSTANCES page #76

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

PaolaDMadd-Pro
Copy link
Contributor

@PaolaDMadd-Pro PaolaDMadd-Pro commented Feb 17, 2025

What?

as per jira ticket EPP-146 Create a CHANGE SUBSTANCES page

Why?

to allow the user to be directed to change substance flow or to skip this step

How?

  • add change-substances.html
  • add fields in index.js, fields.json and pages.json and fiels/index.js
  • add summary-data-section.js
  • add some path to next page

Testing?

n/a manual

Screenshots (optional)

Anything Else? (optional)

Check list

  • I have reviewed my own pull request for linting issues (e.g. adding new lines)
  • I have written tests (if relevant)
  • I have created a JIRA number for my branch
  • I have created a JIRA number for my commit
  • I have followed the chris beams method for my commit https://cbea.ms/git-commit/
    here is an example commit
  • Ensure drone builds are green especially tests
  • I will squash the commits before merging

- add change-substances.html
- add fields in index.js, fields.json and pages.json and fiels/index.js
- add summary-data-section.js
- add some path to next page
@PaolaDMadd-Pro PaolaDMadd-Pro force-pushed the EPP-146-amend-CHANGE-SUBSTANCES branch from 2b1769b to 9980156 Compare February 17, 2025 20:00
@PaolaDMadd-Pro PaolaDMadd-Pro changed the title EPP-146-amend CHANGE SUBSTANCES page EPP-146 amend CHANGE SUBSTANCES page Feb 17, 2025
condition: req =>
req.sessionModel.get('amend-change-substances-options') === 'no'
&& req.sessionModel.get('amend-name-options') === 'yes'
|| req.sessionModel.get('amend-home-address-options') === 'yes'
Copy link
Contributor

@vivekkumar-ho vivekkumar-ho Feb 18, 2025

Choose a reason for hiding this comment

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

I think we should extract these conditions into a function or variable for readability and also reusability in case we would need to use the same conditions on any other route.

condition: req =>
req.sessionModel.get('amend-change-substances-options') === 'no'
&& req.sessionModel.get('amend-name-options') === 'no'
&& req.sessionModel.get('amend-home-address-options') === 'no'
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

Copy link

@william-gu-hof william-gu-hof left a comment

Choose a reason for hiding this comment

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

Had a look through the files, Vivek's comment are good suggestions

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.

3 participants