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

DEV-489: Refactor contributor dashboard to use react-query, hide deleted contribs locally #799

Merged
merged 2 commits into from Nov 10, 2022

Conversation

nrh-cklimas
Copy link
Contributor

Please fill out each section even if it's just with "N/A"

Plan? (if this draft/incomplete indicate what you intend to do and how)

n/a

What's this PR do?

User-facing:

This changes the contributor dashboard so that if you successfully cancel a subscription, that subscription will disappear from the table. However, this update is only temporary. Refreshing the browser or otherwise causing data to be fetched from the API will cause it to reappear. This is a limitation of what we can do with the SPA, since we don't have backend changes yet to support it updating during a user session.

The contribution list is no longer sortable, because it didn't work correctly in the first place (see DEV-2152) and clicking to sort too often caused errors. I'd like to bring that back in a separate story since the PR is big as it is.

Behind the scenes:

This is a refactor of the contributor dashboard because most of the existing components expected to be given an Axios request that resolved to data. This was replaced with a hook, useContributorContributionList. The name is because an admin will get a different set of data fields.

  • Created base components for tables and pagination that are intended to match existing styling.
  • Created a common component, PaymentStatus, that handles formatting of items like "Paid" and "Pending". The existing code had the org admin contributions list import the component from the contributor dashboard.
  • Created a common component, ValueOrPlaceholder, to make the logic of "show children if this value is not nullish, otherwise show ----" reusable.

Why are we doing this? How does it help us?

This improves UX around cancelling a subscription and makes progress toward unifying our code around using react-query for data retrieval.

How should this be manually tested? Please include detailed step-by-step instructions.

To test the change:

  • Create a test recurring contribution.
  • Log into the contributor dashboard.
  • Cancel the contribution.
  • After confirming the cancellation, the subscription should disappear from the table. However, refreshing in your browser will see it come back.
  • Confirm that after cancelling the test subscription, it appears as cancelled in Stripe.

To test regressions (important, because this changes a lot about how the contributor dashboard works behind the scenes):

  • Log into the contributor dashboard.
  • Confirm the table shows accurate data as compared to Stripe.
  • Confirm that the pager buttons at the bottom of the table work.
  • Confirm the table looks the same as on an existing deploy (like dev).
  • Confirm that the pager buttons at the bottom of the table look the same as on an existing deploy.
  • Confirm that editing a credit card on a subscription works.
    • Confirm that in the edit dialog, clicking the last 4 digits of the existing credit card does nothing. (This sounds weird but it uses the same component as in the table, which is editable.)

Did this PR make changes to the user interface that may require changes to the user-facing docs?

It removed the sortable table header in the contributor dashboard temporarily.

Have automated unit tests been added? If not, why?

Yes.

How should this change be communicated to end users?

We might want to explain that sorting the table will return.

Are there any smells or added technical debt to note?

Maintaining local state is not great, but it seems like the best we can do until backend changes happen.

Has this been documented? If so, where?

No.

What are the relevant tickets? Add a link to any relevant ones.

https://news-revenue-hub.atlassian.net/browse/DEV-489

Do any changes need to be made before deployment to production (adding environment variables, for example)?

No.

Are there next steps? If so, what? Have tickets been opened for them? List next-step tickets here:

https://news-revenue-hub.atlassian.net/browse/DEV-2152

@nrh-cklimas nrh-cklimas force-pushed the DEV-489-update-transaction-list-after-cancel branch from c4614c4 to ede8f04 Compare November 1, 2022 17:51
@x110dc x110dc temporarily deployed to rev-engine-dev-489-upda-8mxeh7 November 1, 2022 17:52 Inactive
@cypress
Copy link

cypress bot commented Nov 1, 2022



Test summary

185 0 1 0


Run details

Project NRE
Status Passed
Commit 7196f6b
Started Nov 10, 2022 6:58 PM
Ended Nov 10, 2022 7:03 PM
Duration 05:03 💡
OS Linux Ubuntu - 20.04
Browser Electron 102

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

Copy link
Contributor

@GuiMend GuiMend left a comment

Choose a reason for hiding this comment

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

Looking great Chris. Lot's of structure changes and I liked them 👍

@nrh-cklimas nrh-cklimas force-pushed the DEV-489-update-transaction-list-after-cancel branch from ede8f04 to eb30e43 Compare November 4, 2022 13:24
@x110dc x110dc temporarily deployed to rev-engine-dev-489-upda-8mxeh7 November 4, 2022 13:24 Inactive
@nrh-cklimas nrh-cklimas force-pushed the DEV-489-update-transaction-list-after-cancel branch from eb30e43 to 60be657 Compare November 4, 2022 18:24
@x110dc x110dc temporarily deployed to rev-engine-dev-489-upda-8mxeh7 November 4, 2022 18:24 Inactive
@x110dc x110dc temporarily deployed to rev-engine-dev-489-upda-8mxeh7 November 4, 2022 18:28 Inactive
@x110dc x110dc temporarily deployed to rev-engine-dev-489-upda-8mxeh7 November 7, 2022 18:24 Inactive
@nrh-cklimas nrh-cklimas force-pushed the DEV-489-update-transaction-list-after-cancel branch from 48d91a9 to 7196f6b Compare November 10, 2022 18:56
@x110dc x110dc temporarily deployed to rev-engine-dev-489-upda-8mxeh7 November 10, 2022 18:56 Inactive
@x110dc x110dc merged commit d6ce55b into main Nov 10, 2022
@x110dc x110dc deleted the DEV-489-update-transaction-list-after-cancel branch November 10, 2022 22:09
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.

None yet

4 participants