-
-
Notifications
You must be signed in to change notification settings - Fork 776
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
Cart localstorage update #8485
Cart localstorage update #8485
Changes from all commits
ee973d5
7edf29a
7e59e88
64506ab
a8eb425
e872850
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,18 +11,15 @@ class CartData { | |
static cartContainsGrantWithId(grantId) { | ||
grantId = String(grantId); | ||
const cart = this.loadCart(); | ||
const idList = cart.map(grant => { | ||
return grant.grant_id; | ||
}); | ||
const idList = cart.map((grant) => String(grant.grant_id)); | ||
|
||
return idList.includes(grantId); | ||
} | ||
|
||
static share_url(title) { | ||
const checkedOut = this.loadCheckedOut(); | ||
const donations = (checkedOut.length > 0 ? checkedOut : this.loadCart()); | ||
let bulk_add_cart = 'https://gitcoin.co/grants/cart/bulk-add/'; | ||
|
||
let bulk_add_cart = `${window.location.host}/grants/cart/bulk-add/`; | ||
let network = document.web3network; | ||
|
||
if (!network) { | ||
|
@@ -53,6 +50,10 @@ class CartData { | |
} | ||
|
||
static addToCart(grantData, no_report) { | ||
// Enforce that grant ID is a string, since currently grant IDs are stored as strings in the cart | ||
grantData.grant_id = String(grantData.grant_id); | ||
|
||
// Return if grant is already in cart | ||
if (this.cartContainsGrantWithId(grantData.grant_id)) { | ||
return; | ||
} | ||
|
@@ -246,6 +247,74 @@ class CartData { | |
applyCartMenuStyles(); | ||
} | ||
|
||
// Updates localStorage with the latest grant data from the backend and returns the new cart | ||
static async updateCart() { | ||
// Read cart data from local storage | ||
const cartData = this.loadCart(); | ||
|
||
// Make sure no grants have empty currencies, and if so default to 0.001 ETH. This prevents cart | ||
// from being stuck as 'loading' if a currency is empty | ||
cartData.forEach((grant, index) => { | ||
if (!grant.grant_donation_currency) { | ||
cartData[index].grant_donation_currency = 'ETH'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what about others chains that doesn't have eth as token like zcash and others? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point—so this was copied from But, do we need this anymore? You may recall we had an issue when we changed cart storage (this issue maybe: #8041) and adding defaults was the fix. So we may be able to just delete this though I'm not 100% sure There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ohh ok probably will be handle by the cart tokens logic in that case, but good to make a test with another chain to be sure |
||
cartData[index].grant_donation_amount = '0.001'; | ||
} | ||
}); | ||
|
||
// Get list of all grant IDs and fetch updated grant data from backend | ||
const grantIds = cartData.map((grant) => grant.grant_id); | ||
const url = `${window.location.origin}/grants/v1/api/grants?pks=${grantIds.join(',')}`; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mmm this will add a lot of data into the json delaying more the cart, extending /cart_payload (used on add to cart) to accept multiple ids will give you only the need data + with the props naming already and then you don't even need to add manually the data to cartData you can use just CartData.addToCart(grant) in the foreach. metadata on the json call sometimes on prod is huge, also some queries there to populate other related data on the model add a lot of request time. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you check the diff, you'll notice we were already querying this same endpoint, and we just moved where it happens. So I don't think this will slow anything down compared to the previous baseline? If so that means this is just an optimization then, and I guess my question is do we want to make optimizations on this PR? If so, that's ok, just want to check There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is not the same point not even the same prop names, please check the endpoints on your local with a grant id and you will see the diff just in one grant on local (means without the huge metadata related grants and wall of love) I get a diff of 1.2kb vs 2.7kb so yes will slow a lot. |
||
const response = await fetch(url); | ||
const latestGrantsData = (await response.json()).grants; | ||
|
||
// Update grant info with latest data | ||
cartData.forEach((grant, index) => { | ||
// Find the latestGrantsData entry with the same grant ID as this grant | ||
const grantIndex = latestGrantsData.findIndex((item) => String(item.id) === String(grant.grant_id)); | ||
const latestGrantData = latestGrantsData[grantIndex]; | ||
|
||
// Update with the grant data from server | ||
cartData[index].binance_payout_address = latestGrantData.binance_payout_address; | ||
cartData[index].celo_payout_address = latestGrantData.celo_payout_address; | ||
cartData[index].clr_round_num = latestGrantData.clr_round_num; | ||
cartData[index].grant_admin_address = latestGrantData.admin_address; | ||
cartData[index].grant_clr_prediction_curve = latestGrantData.clr_prediction_curve; | ||
cartData[index].grant_contract_address = latestGrantData.contract_address; | ||
cartData[index].grant_contract_version = latestGrantData.contract_version; | ||
cartData[index].grant_donation_num_rounds = 1; // always 1, since recurring contributions are not supported | ||
cartData[index].grant_id = String(latestGrantData.id); // IDs are saved as strings in localStorage, so we cast to string here | ||
cartData[index].grant_image_css = latestGrantData.image_css; | ||
cartData[index].grant_logo = latestGrantData.logo_url; | ||
cartData[index].grant_slug = latestGrantData.slug; | ||
cartData[index].grant_title = latestGrantData.title; | ||
cartData[index].grant_token_address = latestGrantData.token_address; | ||
cartData[index].grant_token_symbol = latestGrantData.token_symbol; | ||
cartData[index].grant_url = latestGrantData.url; | ||
cartData[index].harmony_payout_address = latestGrantData.harmony_payout_address; | ||
cartData[index].is_clr_eligible = latestGrantData.is_clr_eligible; | ||
cartData[index].is_on_team = latestGrantData.is_on_team; | ||
cartData[index].kusama_payout_address = latestGrantData.kusama_payout_address; | ||
cartData[index].polkadot_payout_address = latestGrantData.polkadot_payout_address; | ||
cartData[index].rsk_payout_address = latestGrantData.rsk_payout_address; | ||
cartData[index].tenants = latestGrantData.tenants; | ||
cartData[index].zcash_payout_address = latestGrantData.zcash_payout_address; | ||
cartData[index].zil_payout_address = latestGrantData.zil_payout_address; | ||
|
||
// Other localStorage properties not updated above: | ||
// grant_donation_amount: Existing localStorage value is preserved | ||
// grant_donation_clr_match: Updated in cart.js `grantData` watcher | ||
// grant_donation_currency: Existing localStorage value is preserved | ||
// payment_status: Updated on-demand in cart.js `updatePaymentStatus()` method | ||
// token_local_id: Existing localStorage value is preserved | ||
// txnid: Updated on-demand in cart.js `updatePaymentStatus()` method | ||
// uuid: TODO is this needed or can it be removed? Currently existing value is preserved | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the UUID used for? what is a grant’s UUID used for? A non-deterministic UUID is generated in In this section of code, should we:
Seems 3 is the way to go, but want to make sure I’m not missing anything first There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @owocki / @zoek1 would you be able to comment on this ? I see the reason it was brought up on #6892 but not clear beyond that ! @mds1 -> if we aren't sure and we know currently it isn't updated , I'd lean towards preserving that just to avoid weird side effects. Let's wait to see what kevin / zoek respond |
||
}); | ||
|
||
// Save the updated cart data to local storage and return it | ||
this.setCart(cartData); | ||
return cartData; | ||
Comment on lines
+273
to
+315
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. all of this could be replaced by There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
} | ||
|
||
static loadCheckedOut() { | ||
const checkedOutList = localStorage.getItem('contributions_were_successful'); | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to change anything about the
bulk-add
endpoint? @octavioamu my understanding was that you wanted to remove it and instead have the Share Cart URL look likehttps://gitcoin.co/grants/cart?ids=<grantIdsList>
. However, I realized one issue with that is the preview links that you see on Twitter would be broken, and would no longer say "3 grants in cart shared by mds1"There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh i think we should still retain the functionality given the bulk-add endpoint has a few extra logic like being able to add grants using
^ This is useful for the Add All to Cart functionality
let's see what @octavioamu was getting at