Skip to content

[QA-1529] Add guard for -1 ids to audius-query api hooks #9647

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.

Sign up for GitHub

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

Jump to bottom
Merged
amendelsohn merged 2 commits into from
Sep 6, 2024
Merged

[QA-1529] Add guard for -1 ids to audius-query api hooks #9647

amendelsohn merged 2 commits into from
Sep 6, 2024

Conversation

@amendelsohn
Copy link
Contributor

@amendelsohn amendelsohn commented Sep 5, 2024

Description

We're getting errors where Id.parse fails on id -1 values. These values shouldn't make it into audius-query fetch in the first place, but adding guards since I wasn't able to track down the caller(s).

I have suspicion about https://github.com/AudiusProject/audius-protocol/blob/main/packages/web/src/components/track/helpers.ts#L16 but wasn't able to trace it to a useGetTrackById usage

How Has This Been Tested?

this is a best effort change

@amendelsohn amendelsohn requested review from a team, schottra and sliptype September 5, 2024 01:00
@changeset-bot
Copy link

changeset-bot bot commented Sep 5, 2024

⚠️ No Changeset found

Latest commit: 1233db4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

schottra
schottra approved these changes Sep 5, 2024
View reviewed changes
Copy link
Contributor

@schottra schottra left a comment

Choose a reason for hiding this comment

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

I'm only just now noticing that this fix is in audius-query, but the source of the
'Something went wrong' messages would be a crashing saga. So just want to confirm this was tested on mobile and fixes the arbitrary toasts.

packages/common/src/api/collection.ts
{ audiusSdk }
) => {
if (!playlistId) return null
if (!playlistId || playlistId === -1) return null
Copy link
Contributor

@schottra schottra Sep 5, 2024

Choose a reason for hiding this comment

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

nit: Might as well do something like id > 0 ? We don't use a constant or anything to mark -1 as special, so we should probably ensure that the values we're using in these cases are valid values.

sliptype
sliptype approved these changes Sep 5, 2024
View reviewed changes
Copy link
Contributor

@sliptype sliptype left a comment

Choose a reason for hiding this comment

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

Ray and I also tracked down a caller passing id -1 by setting a conditional breakpoint inside fetchData I believe. But good to prevent it nonetheless!

@amendelsohn amendelsohn merged commit cdb428a into main Sep 6, 2024
@amendelsohn amendelsohn deleted the am-QA-1529 branch September 6, 2024 16:55
amendelsohn added a commit that referenced this pull request Sep 6, 2024
@amendelsohn
[QA-1529] Add guard for -1 ids to audius-query api hooks (#9647)
07d7ca7
audius-infra pushed a commit that referenced this pull request Sep 7, 2024
Audius Client (Web and Mobile) v1.5.98
73b5ec0 
[34d5a5a] [PAY-3213] ChatListBlastItem mobile UI (#9656) Reed
[518f4d8] fix typecheck on main (#9669) Andrew Mendelsohn
[f183d96] fix type for spl_usdc_payout_wallet on user adapter (#9667) Randy Schott
[dab6070] [QA-1528] Move audius-query force ref tracking to useState (#9661) Andrew Mendelsohn
[d9022cb] fix typecheck in ChatBlastSelectAudienceFields (#9668) Andrew Mendelsohn
[cdb428a] [QA-1529] Add guard for -1 ids to audius-query api hooks (#9647) Andrew Mendelsohn
[ec1dd85] [QA-1542] Fix mobile top supporters crashing profile page (#9646) Andrew Mendelsohn
[7f49a62] [PAY-3416] Adds v1 account endpoint (#9651) Randy Schott
[807d569] [C-5009] Add debug theme (#9611) Dylan Jeffers
[9f75690] [C-5005] Add comment counts (#9630) Dylan Jeffers
[dc74998] [C-4872, C-4993, QA-1538] Add ComposerInput component and update comments and DMs to user the input (#9638) Kyle Shanks
[2bec1f2] [PAY-3420] Update styles on checkout UI (#9649) Raymond Jacobson
[d90eeac] [QA-1545] Fix cover art size on album search results (#9654) Raymond Jacobson
[842a762] Set audienceContentType=track for remix chat blasts (#9655) Reed
[0e6b44e] [C-4975] Comments pagination (#9633) JD Francis
[85fe0e7] Display audience counts in chat blast items (#9640) Reed
[8236a47] [PAY-3401] Consolidate pay with anything tx (#9645) Raymond Jacobson
[f897d8f] [PAY-3411] Enable monetization for everyone when network cut is enabled (#9644) Marcus Pasell
[0902048] Add stems api for v1 (#9513) Raymond Jacobson
[d04bf16] [C-4956] Add mobile comments sort ui (#9637) Sebastian Klingler
[b915249] Fix purchasers count in chat blast modal (#9639) Reed
[adf2207] [PAY-3370] Always use encoded content IDs in chats (#9636) Reed
[53fe54e] [PAY-3388] Migrate feed to SDK (#9593) Randy Schott
[59435ae] Show content title in chat blast item (#9627) Reed
[824fc47] [C-4752] Reduce border width for small Avatar (#9612) Dylan Jeffers
[fa9b498] [C-4952] Mobile comment overflow action confirmations (#9622) Sebastian Klingler
[065e8b8] [PAY-3408][PAY-3409][PAY-3408][PAY-3405][PAY-3413] Update Sales/Purchase Modal (#9624) Marcus Pasell
[04f810a] [PAY-3404] Fix floor() for negative numbers that are already floored (#9623) Marcus Pasell
[fdc7cf3] Fix purchasing without network split (#9629) Marcus Pasell
[682e7d4] [PAY-3410][PAY-3406] Update Sales Table help link and "Value" => "Total" (#9625) Marcus Pasell
[2e621bb] [QA-1439] Don't block gated tracks from being remixes (#9620) Andrew Mendelsohn
[cb753b3] [C-5008] Add single and multi lineHeight options to Text (#9610) Dylan Jeffers
[11911da] [PAY-3378] Disable blast audiences radios with no recipients (#9619) Reed
[826d3ae] Only add blasts to store for sender (#9617) Reed
[661cca7] [PAY-3206] Populate content targeting dropdowns for blast modal (web) (#9601) Andrew Mendelsohn
[f365f01] Add support for expand rounding mode to FixedDecimal (#9606) Marcus Pasell
[d6344cb] Fix web external-text-link style propagation (#9609) Dylan Jeffers
[90b3b41] [PAY-3324] Add copy notice around network take rate (#9608) Marcus Pasell
[d876547] Release mobile apps to 113 (#9614) Dylan Jeffers
[e9091d8] [C-5001] Add comment disabling for tracks (#9594) Dylan Jeffers
[66b1e99] Fix broken common tests (#9605) Marcus Pasell
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@schottra schottra schottra approved these changes

@sliptype sliptype sliptype approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

size/S

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@amendelsohn @schottra @sliptype