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

fix: sort is not always an array #7763

Merged
merged 3 commits into from
Oct 23, 2023
Merged

fix: sort is not always an array #7763

merged 3 commits into from
Oct 23, 2023

Conversation

daiagi
Copy link
Contributor

@daiagi daiagi commented Oct 21, 2023

Thank you for your contribution to the KodaDot - One Stop Shop for Polkadot NFTs.

👇 __ Let's make a quick check before the contribution.

PR Type

  • Bugfix

Context

2023-10-21_15-04

Copilot Summary

🤖 Generated by Copilot at 9b92bd8

Fixed a bug in useChainRedirect.ts that caused an invalid sort parameter to be passed to the API when redirecting from a remark prefix. Added a type check and a conversion logic for the restOfQuery.sort value.

🤖 Generated by Copilot at 9b92bd8

restOfQuery.sort
Array or string? Check first
Autumn leaves no bugs

@daiagi daiagi requested a review from a team as a code owner October 21, 2023 08:13
@daiagi daiagi requested review from preschian and roiLeo and removed request for a team October 21, 2023 08:13
@netlify
Copy link

netlify bot commented Oct 21, 2023

Deploy Preview for koda-canary ready!

Name Link
🔨 Latest commit 231ea3e
🔍 Latest deploy log https://app.netlify.com/sites/koda-canary/deploys/653622bfeb7fd800088d8592
😎 Deploy Preview https://deploy-preview-7763--koda-canary.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@reviewpad
Copy link
Contributor

reviewpad bot commented Oct 21, 2023

AI-Generated Summary: This pull request includes a fix for a bug in the useChainRedirect.ts code. This bug assumes that sort is always an array, but this is not always the case. The fix checks first if sort is an array before performing a filter operation on it. Additionally, if sort is not an array, the code checks if it starts with 'instance_' and if it does, then it deletes sort.

@reviewpad reviewpad bot added small Pull request is small waiting-for-review labels Oct 21, 2023
@reviewpad
Copy link
Contributor

reviewpad bot commented Oct 21, 2023

Reviewpad Report

⚠️ Warnings

  • Please link an issue to the pull request

@daiagi daiagi changed the title fix: sort is not allways an array fix: sort is not always an array Oct 21, 2023
Copy link
Contributor

@roiLeo roiLeo left a comment

Choose a reason for hiding this comment

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

this part is still not easy to read, maybe we should consider refactoring or use better naming

composables/useChainRedirect.ts Outdated Show resolved Hide resolved
@daiagi
Copy link
Contributor Author

daiagi commented Oct 23, 2023

this part is still not easy to read, maybe we should consider refactoring or use better naming

you're right.
please have a look again (although you approved already)

@codeclimate
Copy link

codeclimate bot commented Oct 23, 2023

Code Climate has analyzed commit 231ea3e and detected 0 issues on this pull request.

View more on Code Climate.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@preschian preschian requested a review from prury October 23, 2023 09:02
@prury
Copy link
Member

prury commented Oct 23, 2023

i still get error tho, not sure if is the same error because it does not show any info, but is in the exact same line:

image

@daiagi
Copy link
Contributor Author

daiagi commented Oct 23, 2023

i still get error tho, not sure if is the same error because it does not show any info, but is in the exact same line:

not the same error

seems to be metamask related:

metamask disabled:

image

enabled:

image

@prury
Copy link
Member

prury commented Oct 23, 2023

i still get error tho, not sure if is the same error because it does not show any info, but is in the exact same line:

not the same error

ok, thank you and sorry for the trouble

@daiagi daiagi added this pull request to the merge queue Oct 23, 2023
Merged via the queue into main with commit 0ac8faa Oct 23, 2023
11 checks passed
@daiagi daiagi deleted the bug-sort-is-not-always-array branch October 23, 2023 17:26
This was referenced Oct 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
small Pull request is small waiting-for-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants