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

Refactor amplify methods to single file #801

Merged
merged 10 commits into from
Nov 14, 2024
Merged

Conversation

bangbay-bluetiger
Copy link
Contributor

@bangbay-bluetiger bangbay-bluetiger commented Nov 6, 2024

Description

Amplify typically has breaking changes with each version upgrade. To mitigate how much refactoring we have to do with each upgrade, we should use generic helpers and call amplify methods from a single file, as much as possible.

Other changes in this PR:

  • Add a 204 response code for banner not found so it doesn't give an error console output
  • Convert deprecated atob and btoa

Related ticket(s)

CMDCT-4037


How to test

  • Everything in the app works as expected. A short list of everything:
    • all api calls
    • auth
    • timeout
    • template file download
  • Automated tests pass

Notes

Left in uses of AnyObject where there's a TODO to replace them. Because of that, I needed to cast to ReportShape in a couple of places. I put TODOs as a reminder to remove the casting when the AnyObject conversion happens.


Pre-review checklist

  • I have added thorough tests, if necessary
  • I have updated relevant documentation, if necessary
  • I have performed a self-review of my code
  • I have manually tested this PR in the deployed cloud environment

Copy link

codeclimate bot commented Nov 13, 2024

Code Climate has analyzed commit 10b8e9f and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (90% is the threshold).

This pull request will bring the total coverage in the repository to 93.8% (0.0% change).

View more on Code Climate.

Copy link
Contributor

@gmrabian gmrabian 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! ⭐

services/app-api/handlers/banners/fetch.ts Show resolved Hide resolved
@bangbay-bluetiger bangbay-bluetiger merged commit e98bf89 into main Nov 14, 2024
22 checks passed
@bangbay-bluetiger bangbay-bluetiger deleted the refactor-amplify branch November 14, 2024 15:54
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