-
Notifications
You must be signed in to change notification settings - Fork 61
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(service-portal-licenses): Fix async pkpass generation #14969
Conversation
WalkthroughThe changes in Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant PkPassComponent
participant API
participant FeatureFlagClient
User->>PkPassComponent: Open QRCode Modal
PkPassComponent->>FeatureFlagClient: Check Feature Flags
FeatureFlagClient-->>PkPassComponent: Return Feature Flags
PkPassComponent->>API: Request PKPass Link
API-->>PkPassComponent: Return Link or Error
PkPassComponent-->>User: Display QRCode or Error Message
Tip Early Access Features
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 3
Outside diff range and nitpick comments (2)
libs/service-portal/licenses/src/components/QRCodeModal/PkPass.tsx (2)
Line range hint
48-48
: TheuseEffect
hook does not specify all of its dependencies:featureFlagClient.getValue
.Consider adding
featureFlagClient.getValue
to the dependency array of theuseEffect
to adhere to best practices and avoid potential bugs in reactivity.
Line range hint
51-51
: Avoid using template literals if interpolation and special-character handling are not needed.Consider replacing the template literal with a regular string for the feature flag key to improve performance slightly.
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- libs/service-portal/licenses/src/components/QRCodeModal/PkPass.tsx (2 hunks)
Additional Context Used
Biome (4)
libs/service-portal/licenses/src/components/QRCodeModal/PkPass.tsx (4)
51-51: Do not use template literals if interpolation and special-character handling are not needed.
4-8: Some named imports are only used as types.
13-14: All these imports are only used as types.
48-48: This hook does not specify all of its dependencies: featureFlagClient.getValue
Path-based Instructions (1)
libs/service-portal/licenses/src/components/QRCodeModal/PkPass.tsx (1)
Pattern
libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
Additional comments not posted (1)
libs/service-portal/licenses/src/components/QRCodeModal/PkPass.tsx (1)
1-1
: Ensure the newuseMutation
import is utilized effectively.
libs/service-portal/licenses/src/components/QRCodeModal/PkPass.tsx
Outdated
Show resolved
Hide resolved
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.
Actionable comments posted: 3
Outside diff range and nitpick comments (1)
libs/service-portal/licenses/src/components/QRCodeModal/PkPass.tsx (1)
Line range hint
48-48
: Ensure all dependencies are specified in theuseEffect
hook.- useEffect(() => { + useEffect(() => { + // Added missing dependency + }, [featureFlagClient.getValue])
Review Details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- libs/service-portal/licenses/src/components/QRCodeModal/PkPass.tsx (2 hunks)
Additional Context Used
Biome (5)
libs/service-portal/licenses/src/components/QRCodeModal/PkPass.tsx (5)
51-51: Do not use template literals if interpolation and special-character handling are not needed.
4-8: Some named imports are only used as types.
13-14: All these imports are only used as types.
48-48: This hook does not specify all of its dependencies: featureFlagClient.getValue
78-78: This hook does not specify all of its dependencies: handleError
Path-based Instructions (1)
libs/service-portal/licenses/src/components/QRCodeModal/PkPass.tsx (1)
Pattern
libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
Datadog ReportAll test runs ✅ 9 Total Test Services: 0 Failed, 9 Passed Test Services
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14969 +/- ##
=======================================
Coverage 37.13% 37.13%
=======================================
Files 6373 6373
Lines 129703 129703
Branches 36999 36999
=======================================
Hits 48161 48161
Misses 81542 81542
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Sentry.
|
* fix:init * fix:try catch --------- Co-authored-by: Þorkell Máni Þorkelsson <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* fix:init * fix:try catch --------- Co-authored-by: Þorkell Máni Þorkelsson <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
What
useEffect to fix async safari shenanigans
Why
Some threading issue is making the ui funky.
Use useEffect for proper timing
Checklist:
Summary by CodeRabbit
New Features
pkpassUrl
accordingly.Improvements
Bug Fixes