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

fix: only return previous payment id #6210

Merged
merged 4 commits into from
Apr 27, 2023
Merged

fix: only return previous payment id #6210

merged 4 commits into from
Apr 27, 2023

Conversation

foochifa
Copy link
Contributor

@foochifa foochifa commented Apr 27, 2023 •
edited
Loading

Problem

Currently we return the previous payment document, but the document may contain PII information in the webhook array.

Solution

Only return the previous payment id in payments.controller

Breaking Changes

  • No - this PR is backwards compatible

Tests

Check basic payment functionality

  • Make a new payment on a form with a new email
  • Check the network return (it should be 404)
  • Check that the usual payment modal is displayed (not duplicate modal)
  • Complete the payment

Check previous payment functionality

  • Make a new payment on the same form with the same email
  • Check the network return (it should be 200)
  • Check that the payload is just the payment id
  • Check that the duplicate payment modal is displayed, and the link leads to the previous payment above
  • Complete the payment

Check opening payment intent

  • Make a new payment on the same form with the same email
  • Check the network return (it should be 200)
  • Check that the response is just the payment id
  • Check that the duplicate payment modal is displayed, and the link leads to the previous payment above
  • Proceed to pay so that there is a payment intent created but don't pay
  • Go back to the form and try to restart to pay again
  • The duplicate payment modal should show the previous successful payment (and not the one with the open payment intent)

Check basic form functionality

  • make a submission on any email form
  • make a submission on any storage form without payment

@foochifa foochifa requested a review from KenLSM April 27, 2023 08:01
KenLSM
KenLSM approved these changes Apr 27, 2023
View reviewed changes
Copy link
Contributor

@KenLSM KenLSM left a comment

Choose a reason for hiding this comment

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

LGTM

@foochifa foochifa added this pull request to the merge queue Apr 27, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 27, 2023
@foochifa foochifa added this pull request to the merge queue Apr 27, 2023
@KenLSM KenLSM removed this pull request from the merge queue due to a manual request Apr 27, 2023
@KenLSM KenLSM added this pull request to the merge queue Apr 27, 2023
@KenLSM KenLSM removed this pull request from the merge queue due to the queue being cleared Apr 27, 2023
@foochifa foochifa merged commit 319570b into develop Apr 27, 2023
@foochifa foochifa deleted the fix/only-return-prev-payment-id branch April 27, 2023 15:07
@wanlingt wanlingt mentioned this pull request May 3, 2023
Merged
55 tasks
@LinHuiqing
Copy link
Contributor

Might be good idea to add the following tests (added them to release 6.46.0 (#6254)):

Check previous payment functionality (open payment intent)

  • Make a new payment on the same form with the same email
  • Check the network return (it should be 200)
  • Check that the response is just the payment id
  • Check that the duplicate payment modal is displayed, and the link leads to the previous payment above
  • Proceed to pay so that there is a payment intent created but don't pay
  • Go back to the form and try to restart to pay again
  • The duplicate payment modal should show the previous successful payment (and not the one with the open payment intent)

@foochifa
Copy link
Contributor Author

foochifa commented May 3, 2023

okie have updated thanks @LinHuiqing

wanlingt added a commit that referenced this pull request May 3, 2023
@wanlingt @LinHuiqing
@foochifa @dependabot @justynoh @KenLSM @tshuli
Merge pull request #6254 from opengovsg/release_v6.46.0
5d32818 
* fix: only return previous payment id (#6210)

* fix: return payment id string instead of payment doc in controller

* fix: update payment route method and comment

* fix: update FE service

* chore: fix name of test files

* chore(deps-dev): bump @typescript-eslint/eslint-plugin from 5.59.0 to 5.59.1 in /shared (#6185)

chore(deps-dev): bump @typescript-eslint/eslint-plugin in /shared

Bumps [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/eslint-plugin) from 5.59.0 to 5.59.1.
- [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases)
- [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/CHANGELOG.md)
- [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v5.59.1/packages/eslint-plugin)

---
updated-dependencies:
- dependency-name: "@typescript-eslint/eslint-plugin"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix: add payout pending section to individual response page (#6214)

* ci: use different payments secrets for separate staging environments (#6181)

* ci: add new ENV_NAME to create .env file script

* docs: update env vars used in aws systems manager

* fix: update ENV_NAME conditional

* ref: use ENV_SITE_NAME instead of ACTUAL_ENV_NAME

* fix: rename variable

* docs: add example for SSM_PREFIX

* chore(deps-dev): bump @typescript-eslint/parser from 5.59.1 to 5.59.2 in /shared (#6225)

chore(deps-dev): bump @typescript-eslint/parser in /shared

Bumps [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/parser) from 5.59.1 to 5.59.2.
- [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases)
- [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/parser/CHANGELOG.md)
- [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v5.59.2/packages/parser)

---
updated-dependencies:
- dependency-name: "@typescript-eslint/parser"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps-dev): bump @typescript-eslint/eslint-plugin from 5.59.1 to 5.59.2 in /shared (#6224)

chore(deps-dev): bump @typescript-eslint/eslint-plugin in /shared

Bumps [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/eslint-plugin) from 5.59.1 to 5.59.2.
- [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases)
- [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/CHANGELOG.md)
- [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v5.59.2/packages/eslint-plugin)

---
updated-dependencies:
- dependency-name: "@typescript-eslint/eslint-plugin"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix(deps): bump date-fns from 2.29.3 to 2.30.0 in /shared (#6220)

Bumps [date-fns](https://github.com/date-fns/date-fns) from 2.29.3 to 2.30.0.
- [Release notes](https://github.com/date-fns/date-fns/releases)
- [Changelog](https://github.com/date-fns/date-fns/blob/v2.30.0/CHANGELOG.md)
- [Commits](date-fns/date-fns@v2.29.3...v2.30.0)

---
updated-dependencies:
- dependency-name: date-fns
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix: move payment drawer to Payments tab within builder drawer (#6213)

* fix: move payment drawer into builder drawer

* fix: open payment drawer when payment preview box is clicked

* chore: move FormPaymentPreview into PaymentPreview in frontend templates

* chore: update width of payment preview box in builder

* fix: add dirty modal functionality for payment editing holding state

* fix: add check for email mode when rendering payment input drawer

* fix: update payment modal to be fullpage on mobile (#6219)

* Update FormPaymentModal to be fullpage on mobile

* Update FormPaymentResumeModal and DuplicatePaymentModal to be fullpage on mobile

* add new <ButtonGroup /> that supports full width display

* refactor: remove redundant justifycontent styling

* fix: missing paymentReceiptEmail for fetch fallback submissions (#6252)

* chore: bump version to v6.46.0

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Lin Huiqing <[email protected]>
Co-authored-by: Foo Chi Fa <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Justyn Oh <[email protected]>
Co-authored-by: Ken Lee Shu Ming <[email protected]>
Co-authored-by: tshuli <[email protected]>
@wanlingt wanlingt mentioned this pull request May 8, 2023
Merged
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KenLSM KenLSM KenLSM approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@foochifa @LinHuiqing @KenLSM