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

portalicious: strings instead of numbers in query params #6328

Merged
merged 1 commit into from
Dec 20, 2024

Conversation

aberonni
Copy link
Contributor

@aberonni aberonni commented Dec 20, 2024

AB#32359

After some digging I realised this was what was causing a 400 error on the retry payment endpoint (sending a string payment id instead of a number, but TS thought it was a number).

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have added tests wherever relevant
  • I have made sure that all automated checks pass before requesting a review
  • I do not need any deviation from our PR guidelines

@aberonni aberonni added the portalicious [DEPRECATED] Changes related to the Portalicious release label Dec 20, 2024
@Copilot Copilot bot review requested due to automatic review settings December 20, 2024 09:43

Choose a reason for hiding this comment

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

Copilot reviewed 5 out of 16 changed files in this pull request and generated no comments.

Files not reviewed (11)
  • interfaces/Portalicious/src/app/components/registration-page-layout/registration-page-layout.component.ts: Evaluated as low risk
  • interfaces/Portalicious/src/app/pages/project-registration-debit-cards/project-registration-debit-cards.page.ts: Evaluated as low risk
  • interfaces/Portalicious/src/app/components/registration-page-layout/components/registration-menu/registration-menu.component.ts: Evaluated as low risk
  • interfaces/Portalicious/src/app/domains/registration/registration.api.service.ts: Evaluated as low risk
  • interfaces/Portalicious/src/app/domains/metric/metric.api.service.ts: Evaluated as low risk
  • interfaces/Portalicious/src/app/services/auth.service.ts: Evaluated as low risk
  • interfaces/Portalicious/src/app/domains/registration/registration.helper.ts: Evaluated as low risk
  • interfaces/Portalicious/src/app/pages/project-payments/project-payments.page.ts: Evaluated as low risk
  • interfaces/Portalicious/src/app/domains/project/project.api.service.ts: Evaluated as low risk
  • interfaces/Portalicious/src/app/domains/payment/payment.api.service.ts: Evaluated as low risk
  • interfaces/Portalicious/src/app/pages/project-registration-personal-information/project-registration-personal-information.page.ts: Evaluated as low risk
@aberonni aberonni enabled auto-merge (squash) December 20, 2024 10:05
Copy link
Contributor

@PeterSmallenbroek PeterSmallenbroek left a comment

Choose a reason for hiding this comment

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

LGTM, only thing that still looks a bit random to me is the 'readonly'ness of projectId

@aberonni aberonni force-pushed the aberonni/portalicious-query-param-strings branch from cb2e40f to 9fb5697 Compare December 20, 2024 13:59
@aberonni aberonni merged commit e0f93b3 into main Dec 20, 2024
5 checks passed
@aberonni aberonni deleted the aberonni/portalicious-query-param-strings branch December 20, 2024 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
portalicious [DEPRECATED] Changes related to the Portalicious release
Development

Successfully merging this pull request may close these issues.

2 participants