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

ENG-4161 fix(portal): harden create claim and canonical/universal ids #849

Merged
merged 13 commits into from
Oct 2, 2024

Conversation

Vitalsine85
Copy link
Member

@Vitalsine85 Vitalsine85 commented Sep 20, 2024

Affected Packages

Apps

  • portal

Packages

  • 1ui
  • api
  • protocol
  • sdk

Tools

  • tools

Overview

  • Removes the offchain piece when creating a claim to prevent further issues with claims stuck in pending.
  • Because we are removing the offchain piece when creating a claim, we have to rely on getting the vault_id from the event log of the transaction so that we can direct users to the detail page. There was a larger discussion about canonical/universal ids, so we are also changing all routes/links for identity and claim detail pages to leverage the vault_id instead of the uuid from our API. In the special case of user identities, we are still falling back to the identity_id, or users wallet address, as the identifier.
  • Removes all pending checks relating to Claims as there won't be any new pending claims with this pattern.

Scope of changes in regards to links:

  1. Create Claim success button
  2. Create Identity success button
  3. Detail Info Card list section link
  4. Updated link in components related to paginated lists. Active positions on claims, activity feed, claims list, list claims, and tags.
  5. Save and unsave buttons in Save List modal on success
  6. Tags form success button
  7. Updated getAtomLink - used to include link in most row style components
  8. On Identity route, view all positions button
  9. Readonly banners on /readonly/claim, identity, and list

Screen Captures

If applicable, add screenshots or screen captures of your changes.

Declaration

  • I hereby declare that I have abided by the rules and regulations as outlined in the CONTRIBUTING.md

Copy link

linear bot commented Sep 20, 2024

ENG-4161 Portal Canonical/Universal IDs

This is a parent ticket for switching Portal to utilize vault_id instead of uuid throughout our detail pages and links.

This has come up in a few separate discussions recently and in another context today as we were talking about ways to harden our create claim flow by removing the need for a pending state.

There's a lot of benefit to switching to using vault_id — we want to move toward using vault_id in general as a canonical link and would align with other apps that are built on the protocol but may not be using our database (such as i7n).

@github-actions github-actions bot added the fix Fix label Sep 20, 2024
Copy link
Member

@jonathanprozzi jonathanprozzi left a comment

Choose a reason for hiding this comment

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

This looks great -- very thorough

I want to do some additional QA given the surface area, but everything looks solid!

I wonder if there's a way we can grab the txReceipt without it being a useEffect but I remember that this is how we've talked about it previously

@Vitalsine85
Copy link
Member Author

This looks great -- very thorough

I want to do some additional QA given the surface area, but everything looks solid!

I wonder if there's a way we can grab the txReceipt without it being a useEffect but I remember that this is how we've talked about it previously

Good callout, just pushed a fix. Setting lastTx and vaultId once the transaction completion dispatch fires since we have the receipt in that block of code.

@0xjojikun
Copy link
Member

Just did some QAing. Think we need to strip out some of the pending claim checks on claim detail after creating a claim and/or refactor it so it successfully loads. If we redirect immediately after creating the claim, the pending check fails.

Hash from atoms: 0x693647604190c803eb251e1a8111e732a210961467f6fdfb39f266fbaeaa6210
Result for actual hash: 0
ApiError - 404: Not Found https://dev.api.intuition.systems/claim/5548/positions?direction=desc&sortBy=CreatedAt&page=1&limit=10
ApiError - 404: Record not found in the DB https://dev.api.intuition.systems/claims/5548
IDENTITY IS 404
ApiError - 404: Record not found in the DB https://dev.api.intuition.systems/claims/5548
ApiError - 400: Bad Request https://dev.api.intuition.systems/claims/pending/5548
catching pendingError
isPending: false
isPending: false

@Vitalsine85
Copy link
Member Author

Just did some QAing. Think we need to strip out some of the pending claim checks on claim detail after creating a claim and/or refactor it so it successfully loads. If we redirect immediately after creating the claim, the pending check fails.

Hash from atoms: 0x693647604190c803eb251e1a8111e732a210961467f6fdfb39f266fbaeaa6210
Result for actual hash: 0
ApiError - 404: Not Found https://dev.api.intuition.systems/claim/5548/positions?direction=desc&sortBy=CreatedAt&page=1&limit=10
ApiError - 404: Record not found in the DB https://dev.api.intuition.systems/claims/5548
IDENTITY IS 404
ApiError - 404: Record not found in the DB https://dev.api.intuition.systems/claims/5548
ApiError - 400: Bad Request https://dev.api.intuition.systems/claims/pending/5548
catching pendingError
isPending: false
isPending: false

That last commit removes all the pending checks for claims.

await sleep(RETRY_DELAY)
}
} catch (error) {
if (attempt === MAX_RETRIES - 1) {
Copy link
Member

Choose a reason for hiding this comment

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

could error be some other issues from BE? it could be 500 or 400 (we should be expecting a 404 right)?

@jonathanprozzi
Copy link
Member

Nice job @Vitalsine85 !

@Vitalsine85 Vitalsine85 enabled auto-merge (squash) October 2, 2024 16:08
@Vitalsine85 Vitalsine85 merged commit 06cdbd1 into main Oct 2, 2024
3 checks passed
@Vitalsine85 Vitalsine85 deleted the vital/eng-4161-portal-canonicaluniversal-ids branch October 2, 2024 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants