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

feat: new dialog to set RECEIPT as private #404

Merged
merged 3 commits into from
Mar 13, 2024
Merged

Conversation

dq18
Copy link
Collaborator

@dq18 dq18 commented Mar 3, 2024

What

Thanks to #401 there is an API endpoint to update some proof fields. User can now edit their RECEIPT proofs in Dashboard Proof page by opening a dialog.

image
image

This dialog can further include:

  • proof type change
  • proof deletion
  • add related proofs (for batch mode)?

Part of #363

@dq18 dq18 requested a review from raphodn March 3, 2024 19:08
@@ -15,7 +15,9 @@
</v-chip>
<PriceCountChip :count="proof.price_count" :withLabel="true" @click="goToProof()"></PriceCountChip>
<RelativeDateTimeChip :dateTime="proof.created"></RelativeDateTimeChip>
<ProofIsPublicChip v-if="proof.type === 'RECEIPT'" :proof="proof"></ProofIsPublicChip>
Copy link
Member

@raphodn raphodn Mar 3, 2024

Choose a reason for hiding this comment

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

my 2 cents : in terms of UX, I wouldn't mix tags and forms/actions.

  • currently there's info tags on the left, and a delete button on the right
  • i would rather add an extra "edit" button : clicking opens up a dialog, owner can edit both the type & the public/private
  • and maybe put the delete inside it as well in the long term ? (or not, haven't given much thought on it yet)

Moving things to a dialog allows adding extra info to the user :

  • explain what "a private proof" means
  • explain that the "private feature" is only available for receipts (and the toggle could be set to readonly for other proofs)
  • a cancel button to avoid misclicks

Copy link
Member

@raphodn raphodn Mar 3, 2024

Choose a reason for hiding this comment

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

also it's missing some kind of userCanEditProof or isProofOwner

Copy link
Member

Choose a reason for hiding this comment

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

to go even further in the reflexion, this could be 2 PR :

  • 1 to display the public or private information (as an owner & as an other user)
  • 1 to edit this information (if you are the owner)

Copy link
Collaborator Author

@dq18 dq18 Mar 3, 2024

Choose a reason for hiding this comment

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

I agree, we should rethink the proof page and the rest as the app becomes more complex. We are close to needing a big UI/UX research phase.

I am not sure I understand what you mean by:

also it's missing some kind of userCanDeleteProof or isProofOwner

Do you mean we can use those methods outside the proof page?
Right now, only the user can see a proof that he uploaded and that has no price associated (thus can be deleted). A deletable proof is never seen by other users.
When checking a proof associated by a price by clicking the proof chip, we can just return a tooltip saying that it is private if not the owner. This just needs an assertion of proof.owner on click.

Following this PR, 2 more will follow:

  • Set proof as private when adding from a "Add Price Receipt" form. Still hesitating if I should pass it in proofCreate or proofUpdate. Since proof gets uploaded right after selection, the parameter is_public has to be chosen by the user before the selection, which is counter-intuitive. My thought is to add a toggle or checkbox showing up once proof is uploaded to mark it as private.
  • Disable non owners to see a private proof as mentioned above

Copy link
Member

Choose a reason for hiding this comment

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

as mentionned above, there should be a check userCanEditProof
for example, in the latest prices, any user can see the price proofs

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Alright, it's my point 2

Disable non owners to see a private proof as mentioned above

I will start working on that

@@ -29,7 +31,9 @@ export default {
components: {
'PriceCountChip': defineAsyncComponent(() => import('../components/PriceCountChip.vue')),
'RelativeDateTimeChip': defineAsyncComponent(() => import('../components/RelativeDateTimeChip.vue')),
'ProofDeleteChip': defineAsyncComponent(() => import('../components/ProofDeleteChip.vue'))
'ProofDeleteChip': defineAsyncComponent(() => import('../components/ProofDeleteChip.vue')),
'ProofIsPublicChip': defineAsyncComponent(() => import('../components/ProofisPublicChip.vue'))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
'ProofIsPublicChip': defineAsyncComponent(() => import('../components/ProofisPublicChip.vue'))
'ProofIsPublicChip': defineAsyncComponent(() => import('../components/ProofIsPublicChip.vue'))

@@ -15,7 +15,9 @@
</v-chip>
<PriceCountChip :count="proof.price_count" :withLabel="true" @click="goToProof()"></PriceCountChip>
<RelativeDateTimeChip :dateTime="proof.created"></RelativeDateTimeChip>
<ProofIsPublicChip v-if="proof.type === 'RECEIPT'" :proof="proof"></ProofIsPublicChip>
Copy link
Member

Choose a reason for hiding this comment

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

as mentionned above, there should be a check userCanEditProof
for example, in the latest prices, any user can see the price proofs

@dq18 dq18 changed the title feat: clickable proof chip to set RECEIPT as private feat: new dialog to set RECEIPT as private Mar 12, 2024
@dq18 dq18 merged commit 92fbe8d into master Mar 13, 2024
2 checks passed
@dq18 dq18 deleted the update_proof_user_page branch March 13, 2024 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants