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.

Sign up for GitHub

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

Jump to bottom

Feat/prsd 372 landlord registration check answers #93

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Feat/prsd 372 landlord registration check answers #93

wants to merge 15 commits into from

Conversation

Kiran-R-K
Copy link
Contributor

@Kiran-R-K Kiran-R-K commented Dec 9, 2024 •
edited
Loading

  • created the check answers journey step
  • created the LandlordRegistrationCheckAnswersPage to to populate the model that should be returned for this page
  • ensured the page renders the correct data and links according to users input
  • ensured that data is persisted through each journey flow (so if a user changes from being a UK resident and back again the data they input from that flow prepopulates when they return to the pages)
  • updated existing tests and created new ones

Notes:

  • There are not yet integration tests to check the page when a user has had their ID verified by one login - they were proving tricky and I felt it was better to get the PR in while I worked on them. - These have now been pushed to the PR
  • Some references to the check answers page were using CheckAnswersPage in their name and some were using SummaryPage I have changed them all to CheckAnswersPage for consistency.

AEPR
AEPR reviewed Dec 9, 2024
View reviewed changes
Copy link
Contributor

@AEPR AEPR left a comment

Choose a reason for hiding this comment

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

I've only got through the src, not looked at test yet. Looks good so far.

src/main/kotlin/uk/gov/communities/prsdb/webapp/forms/journeys/LaUserRegistrationJourney.kt Show resolved Hide resolved
...n/kotlin/uk/gov/communities/prsdb/webapp/forms/pages/LandlordRegistrationCheckAnswersPage.kt Show resolved Hide resolved
...n/kotlin/uk/gov/communities/prsdb/webapp/forms/pages/LandlordRegistrationCheckAnswersPage.kt Outdated Show resolved Hide resolved
...n/kotlin/uk/gov/communities/prsdb/webapp/forms/pages/LandlordRegistrationCheckAnswersPage.kt Show resolved Hide resolved
...n/kotlin/uk/gov/communities/prsdb/webapp/forms/pages/LandlordRegistrationCheckAnswersPage.kt Show resolved Hide resolved
isobel-softwire
isobel-softwire reviewed Dec 9, 2024
View reviewed changes
Copy link
Contributor

@isobel-softwire isobel-softwire left a comment

Choose a reason for hiding this comment

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

Left some comments, mostly about testing, but this looks great 👍

...n/kotlin/uk/gov/communities/prsdb/webapp/forms/pages/LandlordRegistrationCheckAnswersPage.kt Outdated Show resolved Hide resolved
...n/kotlin/uk/gov/communities/prsdb/webapp/forms/pages/LandlordRegistrationCheckAnswersPage.kt Show resolved Hide resolved
src/test/kotlin/uk/gov/communities/prsdb/webapp/integration/LandlordRegistrationJourneyTests.kt Outdated Show resolved Hide resolved
...n/pageObjects/pages/landlordRegistrationJourneyPages/CheckAnswersPageLandlordRegistration.kt Outdated Show resolved Hide resolved
src/test/kotlin/uk/gov/communities/prsdb/webapp/integration/LandlordRegistrationJourneyTests.kt Outdated Show resolved Hide resolved
...n/pageObjects/pages/landlordRegistrationJourneyPages/CheckAnswersPageLandlordRegistration.kt Outdated Show resolved Hide resolved
...n/kotlin/uk/gov/communities/prsdb/webapp/forms/pages/LandlordRegistrationCheckAnswersPage.kt Outdated Show resolved Hide resolved
...n/kotlin/uk/gov/communities/prsdb/webapp/forms/pages/LandlordRegistrationCheckAnswersPage.kt Outdated Show resolved Hide resolved
...t/kotlin/uk/gov/communities/prsdb/webapp/integration/pageObjects/components/BaseComponent.kt Outdated Show resolved Hide resolved
AEPR
AEPR reviewed Dec 10, 2024
View reviewed changes
Copy link
Contributor

@AEPR AEPR left a comment

Choose a reason for hiding this comment

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

Reviewed test files, nothing to add that @isobel-softwire hasn't already said!

@Kiran-R-K Kiran-R-K force-pushed the feat/PRSD-372-landlord-registration-check-answers branch 2 times, most recently from ed084f7 to 2773962 Compare December 10, 2024 16:00
@Kiran-R-K
Copy link
Contributor Author

Things left to do on this ticket are:
The pipeline is failing on a test that's passing locally - figure out why and fix it
Action this comment: #93 (comment)
Action this comment: #93 (comment)
Tis discussion is ongoing, speak to Travis about it: #93 (comment)

@Kiran-R-K Kiran-R-K force-pushed the feat/PRSD-372-landlord-registration-check-answers branch 2 times, most recently from 51ac16e to ea781a4 Compare December 16, 2024 14:49
isobel-softwire and others added 15 commits December 16, 2024 17:11
@isobel-softwire @Kiran-R-K
PRSD-642: Updates Navigator and moves identity verification into it
b8865c7
@isobel-softwire @Kiran-R-K
PRSD-642: Consolidates LandlordRegistrationJourneyTests and creates t…
f397f0a 
…ests for Confirmation step
@Kiran-R-K
feat/PRSD-372 landlord registration check answers
1bdb188
@Kiran-R-K
feat/PRSD-372 landlord registration check answers
32f4e00
@Kiran-R-K
feat/PRSD-372 landlord registration check answers
04aca61
@Kiran-R-K
PRSD-372: check answers page landlord registration
7f88a03
@Kiran-R-K
PRSD-372 rename SummaryPage to CheckAnswersPage
3cd80a7
@Kiran-R-K
PRSD-372 added tests for check answers when identity verification has…
47aeb6f 
… been completed
@Kiran-R-K
PRSD-372 added tests for check answers when identity verification has…
59252a6 
… been completed
@Kiran-R-K
feat/PRSD-372 review markups
aefb6ae
@Kiran-R-K
PRSD-372 review mark ups
368df5e
@Kiran-R-K
feat/PRSD-372 review mark ups
0b5b0d9
@Kiran-R-K
feat/PRSD-372 created summary list page object
5b96850
@Kiran-R-K
feat/PRSD-372 post merge fix
660bc09
@Kiran-R-K
feat/PRSD-372 post merge fix
15c5b28
@Kiran-R-K Kiran-R-K force-pushed the feat/PRSD-372-landlord-registration-check-answers branch from 42e6552 to 15c5b28 Compare December 16, 2024 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@isobel-softwire isobel-softwire isobel-softwire left review comments

@Travis-Softwire Travis-Softwire Travis-Softwire left review comments

@AEPR AEPR AEPR left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Kiran-R-K @AEPR @Travis-Softwire @isobel-softwire