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

PP-12854 Parity check - omit unnecessary fields and handle connector 404 #1844

Merged
merged 6 commits into from
Sep 19, 2024

Conversation

james-peacock-gds
Copy link
Contributor

@james-peacock-gds james-peacock-gds commented Sep 16, 2024

Context: The parity check in toolbox compares responses from connector and ledger in relation to a given transaction. It should only check the fields that are common to both responses, highlighting any missing or different fields. If connector returns 404 (charge not found), a message should be displayed rather than an error page.

  • Omit some fields which are specific to connector or ledger (i.e. those which are not common to both).
  • Handle a 404 from connector by way of a message on the parity check page rather than presenting an error page.
  • Improve layout so that differences in values, and missing fields, are shown under separate sub-headings

Screenshot: Charge not found in connector:

Screenshot 2024-09-18 at 17 50 55

Screenshot: Differences found:

Screenshot 2024-09-18 at 17 52 17

@james-peacock-gds james-peacock-gds changed the title PP-12854 Omit additional fields from parity check PP-12854 Parity check - omit unnecessary fields and handle connector 404 Sep 17, 2024
- correct the diff filtering logic so that relevant differences are all included
- lay out page so that the diff kinds (different fields, missing fields) are under sub-headings
Copy link
Collaborator

@nlsteers nlsteers left a comment

Choose a reason for hiding this comment

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

Nice work on this, a few comments to consider

text: warningMessage,
{% if message %}
{{ govukInsetText({
text: message,
iconFallbackText: "Warning"
Copy link
Collaborator

Choose a reason for hiding this comment

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

this iconFallbackText parameter is probably not needed anymore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done!

@james-peacock-gds james-peacock-gds merged commit 15de133 into master Sep 19, 2024
8 checks passed
@james-peacock-gds james-peacock-gds deleted the PP-12854_ledger-parity-display branch September 19, 2024 16:35
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