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

HPC-9578 Add Apollo Client #462

Open
wants to merge 2 commits into
base: HPC-9502
Choose a base branch
from
Open

HPC-9578 Add Apollo Client #462

wants to merge 2 commits into from

Conversation

Onitoxan
Copy link
Contributor

@Onitoxan Onitoxan commented Aug 21, 2024

Nature of PR

  • Bug-fix
  • Hot-fix
  • Feature
  • Testing
  • Rewrite
  • CI
  • Package update

Description

Add Apollo client to make calls to GrapQL endpoints. Beside setting up Apollo client I added an AbortSignal for calls that are using our REST endpoints, like this when changing of page the petition gets cancelled.

Out of the box Apollo will let us have a cache system, that's why I remove the abort signals from petitions using it, once they are made they will not repeat themselves and they will be accessed form the cache.

Apollo allow use to use their own hooks like useQuery, but since we have already implemented in the project our own dataLoader I was not sure what would be the best idea. Also, reusing our dataLoader makes the implementation easier with less changes.

How to test changes

To test changes I feel the key places to look are:

  1. model.ts I added Apollo client here and we should double check that this should be the place where it belongs, if it's not the case we need to see where would that be.
  2. Navigate through Flow and Pending Flow pages and play around with searches to verify that the cache policy is no making as return false data.

TODO:

Maybe we could investigate what else could we do with Apollo Client, there is a TODO comment in the code that we could look into:

TODO: Dynamically fetch only necessary fields, Ex: if we don't display 'NewMoney' we shouldn't ask for it

And I suppose Apollo offers some kind of solution for this. In any case, if we feel is the case we could open a ticket in the future.

@Onitoxan Onitoxan added the ready for review All comments have been addressed, and the Pull Request is ready for review label Aug 21, 2024
@Onitoxan Onitoxan requested a review from a team as a code owner August 21, 2024 13:51
Copy link

@unocha-hpc unocha-hpc left a comment

Choose a reason for hiding this comment

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

Checks have passed and this pull request is ready for manual review

Copy link

@unocha-hpc unocha-hpc left a comment

Choose a reason for hiding this comment

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

Checks have passed and this pull request is ready for manual review

Copy link

@unocha-hpc unocha-hpc left a comment

Choose a reason for hiding this comment

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

Checks have passed and this pull request is ready for manual review

Copy link

@unocha-hpc unocha-hpc left a comment

Choose a reason for hiding this comment

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

Checks have passed and this pull request is ready for manual review

Copy link

@unocha-hpc unocha-hpc left a comment

Choose a reason for hiding this comment

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

Checks have passed and this pull request is ready for manual review

Copy link

@unocha-hpc unocha-hpc left a comment

Choose a reason for hiding this comment

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

Checks have passed and this pull request is ready for manual review

Copy link

@unocha-hpc unocha-hpc left a comment

Choose a reason for hiding this comment

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

Checks have passed and this pull request is ready for manual review

Copy link

@unocha-hpc unocha-hpc left a comment

Choose a reason for hiding this comment

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

Checks have passed and this pull request is ready for manual review

Copy link
Contributor

@manelcecs manelcecs left a comment

Choose a reason for hiding this comment

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

Everything looks good!

We'll need to rebase this branch with all the new work and previously merged work too.

The only thing pending is to do a pair session with you to review the functionality and next steps to do (such as what can do Apollo Server to help to reduce the payload or to maintain the same data structure/schema)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for review All comments have been addressed, and the Pull Request is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants