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

23968 Business Dashboard UI: implement background retry to check for latest state #117

Merged
merged 19 commits into from
Jan 17, 2025

Conversation

Rajandeep98
Copy link
Collaborator

@Rajandeep98 Rajandeep98 commented Jan 10, 2025

*Issue:*bcgov/entity#23968

Description of changes:

every 1 second until the 10 second mark
every 10 seconds until the 1 minute mark
every minute until the 30 minute mark
every hour thereafter
data polling slim end point , compare it with initial /identifier api endpoint's modified date,
show toast msg if modified date vs slim endpoint modified date > 0s

@Rajandeep98
Copy link
Collaborator Author

/gcbrun

@bcregistry-sre
Copy link
Collaborator

bcregistry-sre commented Jan 10, 2025

@severinbeauvais
Copy link
Collaborator

severinbeauvais commented Jan 10, 2025

Please update the app version before merging. done

Copy link
Collaborator

@severinbeauvais severinbeauvais left a comment

Choose a reason for hiding this comment

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

Something isn't working right. I tested using the preview link and I don't see the 10 x 1-second polls. I only see the 10-second polls to start. code will be updated

@severinbeauvais
Copy link
Collaborator

severinbeauvais commented Jan 10, 2025

deleted obsolete comment

@Rajandeep98
Copy link
Collaborator Author

/gcbrun

@Rajandeep98 Rajandeep98 reopened this Jan 13, 2025
Copy link
Collaborator

@severinbeauvais severinbeauvais left a comment

Choose a reason for hiding this comment

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

Still to do:

  • check unresolved comment in dashboard.vue resolved
  • check unresolved comment in business.ts resolved
  • update app version resolved
  • update screenshots resolved

@Rajandeep98
Copy link
Collaborator Author

image

@Rajandeep98
Copy link
Collaborator Author

/gcbrun

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://business-dashboard-dev--pr-117-n0kartyb.web.app

@Rajandeep98
Copy link
Collaborator Author

/gcbrun

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://business-dashboard-dev--pr-117-n0kartyb.web.app

@severinbeauvais
Copy link
Collaborator

There's a new Cypress test error above. Can you look into it?

cc: @patrickpeinanw , any ideas?

@severinbeauvais
Copy link
Collaborator

/gcbrun

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://business-dashboard-dev--pr-117-n0kartyb.web.app

@patrickpeinanw
Copy link
Collaborator

There's a new Cypress test error above. Can you look into it?

cc: @patrickpeinanw , any ideas?

@Rajandeep98 the error message is "Cannot read properties of null (reading 'lastModified')". In some cases slimBusiness or currentBusiness.value is null.

To run and debug cypress tests locally, you first build and run the app (pnpm run build:local && pnpm run preview), then do pnpm run test:e2e

@Rajandeep98
Copy link
Collaborator Author

/gcbrun

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://business-dashboard-dev--pr-117-n0kartyb.web.app

@Rajandeep98
Copy link
Collaborator Author

/gcbrun

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://business-dashboard-dev--pr-117-n0kartyb.web.app

@Rajandeep98
Copy link
Collaborator Author

/gcbrun

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://business-dashboard-dev--pr-117-n0kartyb.web.app

@severinbeauvais severinbeauvais self-requested a review January 17, 2025 17:26
Copy link
Collaborator

@JazzarKarim JazzarKarim left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Can you please update the package.json version please?

Copy link
Collaborator

@severinbeauvais severinbeauvais left a comment

Choose a reason for hiding this comment

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

Looks good. Merge when ready.

You're fixing the Cypress tests in a future ticket or PR, right?

@Rajandeep98
Copy link
Collaborator Author

re-run cypress, all tests passed

@Rajandeep98 Rajandeep98 merged commit d103e71 into bcgov:main Jan 17, 2025
5 of 6 checks passed
@severinbeauvais
Copy link
Collaborator

@Rajandeep98 , thanks for merging. I forgot to tell you: use the "squash" option when merging so that your PR gets merged as a single commit instead of multiple invididual commits.

@severinbeauvais severinbeauvais changed the title Business Dashboard UI: implement background retry to check for latest state 23968 Business Dashboard UI: implement background retry to check for latest state Jan 17, 2025
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.

5 participants