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

[cms] feat: switch proposal token to proposal name on invoice lineitem #2041

Merged

Conversation

victorgcramos
Copy link
Member

@victorgcramos victorgcramos commented Jul 2, 2020

This diff Closes #2016 and changes the lineitem selector for proposal token to proposal name.

Solution description

Creates a useApprovedProposals hook to manage approved proposals when needed. Now, invoices send a batch proposals request to fetch proposals according to the lineitem's token.

On InvoiceForm, when selecting the proposal token lineitem, we do a tokeninventory request to fetch all proposals tokens, and then batch the approved ones to get the corresponding proposal.

Use Cases

Now, useApprovedProposals hook can be used when fetching approved proposals is needed.

Dependencies

Depends on decred/politeia#1242

UI Changes Screenshot

before:
Screen Shot 2020-07-02 at 7 36 19 PM

after:
Screen Shot 2020-07-02 at 7 33 51 PM

@victorgcramos victorgcramos changed the title [wip] cms: proposal list on lineitems by name [wip][cms] feat: switch proposal token to proposal name on invoice lineitem Jul 2, 2020
@victorgcramos victorgcramos changed the title [wip][cms] feat: switch proposal token to proposal name on invoice lineitem [cms] feat: switch proposal token to proposal name on invoice lineitem Jul 6, 2020
@victorgcramos victorgcramos marked this pull request as ready for review July 6, 2020 12:11
Copy link
Member

@thi4go thi4go left a comment

Choose a reason for hiding this comment

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

Hey mate good job!

Was getting a CSRF token issue, but then switched to decred/politeia#1242 and it is working fine now. 👍

@victorgcramos
Copy link
Member Author

victorgcramos commented Jul 7, 2020

after moving the batch proposals csrf issue fix to decred/politeia#1242, we now depend on it instead of decred/politeia#1224

Copy link
Member

@alexlyp alexlyp left a comment

Choose a reason for hiding this comment

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

tACK

@amass01
Copy link
Member

amass01 commented Jul 7, 2020

Waiting on decred/politeia#1242

Copy link
Member

@tiagoalvesdulce tiagoalvesdulce left a comment

Choose a reason for hiding this comment

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

I think the useApprovedProposals should have its own file on src/hooks/api and the useProposalsBatch should be moved to its own file there as well. Saying that because we want to keep shared stuff outside politeia and cms containers.

This diff now allows users to select proposals by its name, instead of
its token.

Since getting proposals names require a batching request, and those
requests are bounded by 20 proposals for each request, those requests
have to be done more than once in case the token from the
invoice (details and diff), or
the proposal (new and edit actions) are not on the dropdown list.
@victorgcramos victorgcramos force-pushed the 2016-cms-proposal-list-invoice branch from 746c501 to a689f16 Compare July 8, 2020 14:16
Copy link
Member

@tiagoalvesdulce tiagoalvesdulce left a comment

Choose a reason for hiding this comment

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

Good job, will merge when decred/politeia#1242 gets in.

- add onFetchRemainingProposalsBatch method to fetch remaining
  tokens and lazy load proposal list
Copy link
Member

@alexlyp alexlyp left a comment

Choose a reason for hiding this comment

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

tACK

@tiagoalvesdulce tiagoalvesdulce merged commit ca866ff into decred:master Jul 23, 2020
@victorgcramos victorgcramos deleted the 2016-cms-proposal-list-invoice branch February 15, 2021 19:36
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.

[cms] Change proposal dropdowns to Proposal Title
5 participants