[PAY-1573] Convert premium content type to union #3711
Conversation
| : null | ||
| const nftCollection = isPremiumContentCollectibleGated(premiumConditions) | ||
| ? premiumConditions?.nft_collection | ||
| : null | ||
|
|
There was a problem hiding this comment.
I think it's because of ?? {} ; if we had a branch that checked if premiumConditions was null first and then returned early we'd probably be good, tho would be tricky given hooks here
There was a problem hiding this comment.
still getting type errors even if I null check first. gonna stick with the type guard for now but let's chat about it tmr.
There was a problem hiding this comment.
I can live with the type guards!
| : null | ||
| const nftCollection = isPremiumContentCollectibleGated(premiumConditions) | ||
| ? premiumConditions?.nft_collection | ||
| : null | ||
|
|
There was a problem hiding this comment.
I think it's because of ?? {} ; if we had a branch that checked if premiumConditions was null first and then returned early we'd probably be good, tho would be tricky given hooks here
| const shouldDisplay = | ||
| !!premiumConditions.nft_collection || followee || tippedUser | ||
| isPremiumContentCollectibleGated(premiumConditions) || |
There was a problem hiding this comment.
Eek, somehow made it more verbose.
I would have thought that if our type is a union of types that are all valid, we just need to check if it's not null here?
There was a problem hiding this comment.
ah good point. Probably good to keep it this way for now tho bc usdc_purchase is a valid option but I haven't written the logic to exclude that yet?
| <View> | ||
| <Text style={styles.ownersOf}>{messages.ownersOf}</Text> | ||
| <View style={styles.collection}> | ||
| {premiumConditions.nft_collection.imageUrl && ( | ||
| {premiumConditions.nft_collection?.imageUrl && ( |
There was a problem hiding this comment.
Is the ? necessary here? I would expect that the type guard function narrows it to a type here where we know that nft_collection is defined.
There was a problem hiding this comment.
Unfortunately @saliou and I discovered that nft_collection specifically can be undefined, bc during upload flow user could set the track to collectible gated without specifying a collection yet. Kind of awkward because at every point afterwards all the fields should be defined.
There was a problem hiding this comment.
will include comment
There was a problem hiding this comment.
Ouch. That does make this whole change a little less useful.
There was a problem hiding this comment.
yeah it was a bummer :( couldn't think of a way around it. still gonna be useful for usdc tho.
packages/web/src/components/track-availability-modal/TrackAvailabilityModal.tsx
Show resolved
Hide resolved
|
Preview this change https://demo.audius.co/rt-prem-type-2 |
There was a problem hiding this comment.
slightly more verbose but conditions look cleaner imo.
my only worry is the question marks e.g. premiumConditions.nft_collection?.name which used to be premiumConditions.nft_collection.name, meaning it is theoretically possible that some component not display text when it's supposed to be. We should be careful with it and hopefully find a way to avoid it in the future. not sure just how yet
…1560-usdc-tiles * origin/gated-content-updates: [PAY-1573] Convert premium content type to union (#3711)
Description
Check the type of a
premiumConditionsby using theinoperator instead of whether or not a field likenft_collectionexists inpremiumConditions. Also included some typeguards without which ternaries complain.Dragons
Is there anything the reviewer should be on the lookout for? Are there any dangerous changes?
How Has This Been Tested?
Tested by uploading/attempting to edit collectible + SA tracks on both web and mobile.
How will this change be monitored?
For features that are critical or could fail silently please describe the monitoring/alerting being added.
Feature Flags
Are all new features properly feature flagged? Describe added feature flags.