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

Transactions Page redesign follow-up #3385

Closed
6 of 7 tasks
kewitz opened this issue Aug 4, 2020 · 4 comments
Closed
6 of 7 tasks

Transactions Page redesign follow-up #3385

kewitz opened this issue Aug 4, 2020 · 4 comments
Assignees
Labels
api Issues that require some work on the API (https://github.com/opencollective/opencollective-api) enhancement feedback frontend technical-debt Deprecated code to migrate and other necessary refactors

Comments

@kewitz
Copy link
Contributor

kewitz commented Aug 4, 2020

The Transactions page redesign was shipped yesterday and the old issues were closed.
This is a follow up with improvements and missing tasks.

  • Add server-side rendering.
  • Refactor Transaction export endpoints from V1 to V2 and remove leaking ids to the front-end.
  • Restore the "Refund Button".
  • Review conditionals for accessing the "Download Receipts" button.
  • Resolve Transactions access and permissions in the back-end.
  • Fix comments count bug: https://sentry.io/organizations/open-collective/issues/1822712057/?project=1736806&referrer=slack
  • Update the collective page Budget section to use the V2 endpoint and the new TransactionDetail component.
@kewitz kewitz added api Issues that require some work on the API (https://github.com/opencollective/opencollective-api) frontend enhancement technical-debt Deprecated code to migrate and other necessary refactors feedback labels Aug 4, 2020
@kewitz kewitz self-assigned this Aug 4, 2020
@Betree
Copy link
Member

Betree commented Aug 4, 2020

Since we need to review them. it would be great to have the permissions checks for refund and download receipts server-side as proposed in opencollective/opencollective-frontend#4719 (comment).

@kewitz
Copy link
Contributor Author

kewitz commented Aug 5, 2020

@Betree I think these permissions are actually related to the collective and not transactions.
Perhaps we should refactor the permission check for the collectives in another PR? WDYT?

@Betree
Copy link
Member

Betree commented Aug 6, 2020

While most of the checks are indeed done on the collective(s), I think they still belong conceptually to Transactions because refunding and downloading receipts are an action that you do on transactions, not on collectives. Also because there are still some checks on transactions that can impact the result:

canRefundTransaction(transaction, user) {
  return !transaction.RefundTransactionId || user.isAdmin(transaction.collective.host.id);
}

canDownloadReceipt(transaction, user) {
  if (transaction.UsingVirtualCardFromCollectiveId) {
    return user.isAdmin(transaction.UsingVirtualCardFromCollectiveId);
  } else {
    return user.isAdmin(transaction.FromCollectiveId);
  }
}

@kewitz
Copy link
Contributor Author

kewitz commented Aug 14, 2020

Refactor Transaction export endpoints from V1 to V2 and remove leaking ids to the front-end is more complex than first thought since it interacts with outside services that still rely on the UUID.

@kewitz kewitz closed this as completed Aug 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Issues that require some work on the API (https://github.com/opencollective/opencollective-api) enhancement feedback frontend technical-debt Deprecated code to migrate and other necessary refactors
Projects
None yet
Development

No branches or pull requests

2 participants