-
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(skilavottord): Error when trying to view and save recycling companies #16159
fix(skilavottord): Error when trying to view and save recycling companies #16159
Conversation
WalkthroughThe pull request introduces modifications to the Changes
Possibly related PRs
Suggested reviewers
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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
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: 0
🧹 Outside diff range and nitpick comments (2)
apps/skilavottord/web/screens/RecyclingCompanies/RecyclingCompanyUpdate/RecyclingCompanyUpdate.tsx (1)
118-120
: Approve change with suggestions for improvementThis change effectively addresses the PR objective by preventing the backend error when saving a recycling company. Good job on identifying and fixing this issue!
However, I have a couple of suggestions to further improve the code:
- For better performance, consider using object destructuring instead of the
delete
operator:const { __typename, ...inputWithoutTypename } = input;
- To enhance type safety, explicitly type the
input
object:const handleUpdateRecyclingPartner = handleSubmit(async (input: RecyclingPartnerInput) => { const { __typename, ...inputWithoutTypename } = input; // Use inputWithoutTypename in the mutation // ... });These changes will maintain the functionality while improving performance and type safety.
🧰 Tools
Biome
[error] 119-119: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
apps/skilavottord/web/screens/RecyclingCompanies/RecyclingCompanies.tsx (1)
75-77
: Approved: Path duplication issue resolvedThe removal of
BASE_PATH
from thepathname
inrouter.push
is a good fix for the path duplication issue. This change aligns with NextJS best practices by simplifying the routing logic.Consider updating the inline comment to provide more context:
- pathname: routes.recyclingCompanies.edit, // with BASE-PATH it duplicates the path + pathname: routes.recyclingCompanies.edit, // Removed BASE_PATH to prevent path duplicationThis change makes the comment more explicit about the fix that was implemented.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- apps/skilavottord/web/screens/RecyclingCompanies/RecyclingCompanies.tsx (1 hunks)
- apps/skilavottord/web/screens/RecyclingCompanies/RecyclingCompanyUpdate/RecyclingCompanyUpdate.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
apps/skilavottord/web/screens/RecyclingCompanies/RecyclingCompanies.tsx (1)
Pattern
apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/skilavottord/web/screens/RecyclingCompanies/RecyclingCompanyUpdate/RecyclingCompanyUpdate.tsx (1)
Pattern
apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
Biome
apps/skilavottord/web/screens/RecyclingCompanies/RecyclingCompanyUpdate/RecyclingCompanyUpdate.tsx
[error] 119-119: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments not posted (1)
apps/skilavottord/web/screens/RecyclingCompanies/RecyclingCompanies.tsx (1)
Line range hint
1-329
: Overall adherence to coding guidelinesThis component demonstrates good adherence to the provided coding guidelines:
- It follows NextJS best practices for file structure and routing.
- It efficiently manages state using React hooks and Apollo Client for GraphQL queries.
- It implements server-side rendering techniques through the use of
useQuery
from Apollo Client.- The component and its utilities are properly typed with TypeScript, ensuring type safety.
The recent change improves the routing logic, further aligning the code with NextJS best practices.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #16159 +/- ##
==========================================
- Coverage 36.69% 36.67% -0.03%
==========================================
Files 6776 6769 -7
Lines 139578 139289 -289
Branches 39678 39575 -103
==========================================
- Hits 51222 51087 -135
+ Misses 88356 88202 -154
Flags with carried forward coverage won't be shown. Click here to find out more. see 50 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 6 Total Test Services: 0 Failed, 5 Passed Test Services
|
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.
lgtm 💯
…nies (#16159) - Fix path when trying to view recycling company - Fix error when trying to save recycling company Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
* chore(skilavottord): Inspect why traffic endpoint is not always returning data (#16151) - Added loggers - make sure Traffic is only called when needed Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> * fix(skilavottord): Error when trying to view and save recycling companies (#16159) - Fix path when trying to view recycling company - Fix error when trying to save recycling company Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
…nies (#16159) - Fix path when trying to view recycling company - Fix error when trying to save recycling company Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
TS-920
What
Checklist:
Summary by CodeRabbit