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

Brp-61 - Additional Application type question on all BRP forms #719

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

Rhodine-orleans-lindsay
Copy link
Contributor

@Rhodine-orleans-lindsay Rhodine-orleans-lindsay commented May 3, 2022

What?

  • Add additional question on application type to routes
  • Include application type in email header
  • Amended acceptance and tests to reflect changes

Why?

  • The addition of this question will allow the business to pinpoint if there is an issue with a particular type of BRP application

How?

  • Added additional question to all routes
  • Added additional section to check answers page to reflect additional question
  • Amended email behaviour

Testing?

Tested locally

Screenshots (optional)

Anything Else?

@Rhodine-orleans-lindsay Rhodine-orleans-lindsay changed the title Bro-61 Brp-61 May 3, 2022
@Rhodine-orleans-lindsay Rhodine-orleans-lindsay force-pushed the BRP-61 branch 17 times, most recently from 3697064 to 2763851 Compare May 10, 2022 10:31
@Rhodine-orleans-lindsay Rhodine-orleans-lindsay changed the title Brp-61 Brp-61 - Additional Application type question on all BRP forms May 10, 2022
};
},
'/someone-else': data => {
return {
template: 'someone-else',
subject: 'Form submitted: Report someone else collecting your BRP. Ref: ' + setRef(data)
subject: 'Form submitted: Report someone else collecting your BRP. Ref: ' + setRef(data) +
' (Application Type: ' + appType(data) + ')'
};
}
};
Copy link
Contributor

Choose a reason for hiding this comment

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

this looks good, perhaps you can just talk me through it :)

Copy link
Contributor

@sulthan-ahmed sulthan-ahmed left a comment

Choose a reason for hiding this comment

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

this looks great @rhodine I'll be honest, it's super long, it might be easy just to go through it quickly in a call :)

@Rhodine-orleans-lindsay Rhodine-orleans-lindsay force-pushed the BRP-61 branch 4 times, most recently from 032cf4d to a3a72c3 Compare May 12, 2022 11:53
});
});
});
});
Copy link
Contributor

Choose a reason for hiding this comment

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

oooh nice work @rhodine I prefer these tests rather than lots of acceptance tests which are slow and cause build issues ⭐

Copy link
Contributor

@sulthan-ahmed sulthan-ahmed left a comment

Choose a reason for hiding this comment

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

Great work. This is more of a question? Do we want to be testing all these validators? Is it overkill? We are just essentially adding config and that code has been tested at the HOF level. 🙂

@Rhodine-orleans-lindsay Rhodine-orleans-lindsay force-pushed the BRP-61 branch 13 times, most recently from 935ea13 to 4cf2974 Compare May 24, 2022 13:33
@Rhodine-orleans-lindsay Rhodine-orleans-lindsay force-pushed the BRP-61 branch 4 times, most recently from 9e682e9 to 8be2978 Compare May 30, 2022 12:47
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