-
-
Notifications
You must be signed in to change notification settings - Fork 4
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: data portal saving #1265
fix: data portal saving #1265
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe updates across various packages primarily enhance API functionality, improve code clarity, and ensure better flow control. Notable changes include the integration of new functions and modifications to existing schemas to support more robust data handling and validation. Additionally, adjustments in middleware and error handling aim to streamline operations and improve user interactions within the system. Changes
These changes collectively aim to refine the system's robustness and user experience by enhancing functionality and streamlining processes. 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 (
|
#365 Bundle Size — 3.37MiB (+0.03%).Warning Bundle contains 2 duplicate packages – View duplicate packages Bundle metrics
|
Current #365 |
Baseline #350 |
|
---|---|---|
Initial JS | 2.98MiB (+0.03% ) |
2.98MiB |
Initial CSS | 9.54KiB |
9.54KiB |
Cache Invalidation | 44.05% |
63.28% |
Chunks | 67 (-1.47% ) |
68 |
Assets | 80 (-1.23% ) |
81 |
Modules | 1969 |
1969 |
Duplicate Modules | 270 |
270 |
Duplicate Code | 8.01% (+0.13% ) |
8% |
Packages | 207 |
207 |
Duplicate Packages | 2 |
2 |
Bundle size by type 1 change
1 regression
Current #365 |
Baseline #350 |
|
---|---|---|
JS | 3.25MiB (+0.03% ) |
3.25MiB |
Fonts | 94.54KiB |
94.54KiB |
Other | 9.61KiB |
9.61KiB |
CSS | 9.54KiB |
9.54KiB |
IMG | 8.57KiB |
8.57KiB |
Bundle analysis report Branch JoeKarow/org-level-website-savin... Project dashboard
📦 Next.js Bundle Analysis for @weareinreach/appThis analysis was generated by the Next.js Bundle Analysis action. 🤖
|
Page | Size (compressed) |
---|---|
global |
404.85 KB (🟡 +3.29 KB) |
Details
The global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster.
Any third party scripts you have added directly to your app using the <script>
tag are not accounted for in this analysis
If you want further insight into what is behind the changes, give @next/bundle-analyzer a try!
Two Pages Changed Size
The following pages changed size from the code in this PR compared to its base branch:
Page | Size (compressed) | First Load | % of Budget (575 KB ) |
---|---|---|---|
/org/[slug] |
321.55 KB |
726.4 KB | 126.33% (🟢 -0.56%) |
/org/[slug]/edit |
274.62 KB |
679.47 KB | 118.17% (🟢 -0.56%) |
Details
Only the gzipped size is provided here based on an expert tip.
First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If next/link
is used, subsequent page loads would only need to download that page's bundle (the number in the "Size" column), since the global bundle has already been downloaded.
Any third party scripts you have added directly to your app using the <script>
tag are not accounted for in this analysis
The "Budget %" column shows what percentage of your performance budget the First Load total takes up. For example, if your budget was 100kb, and a given page's first load size was 10kb, it would be 10% of your budget. You can also see how much this has increased or decreased compared to the base branch of your PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this. If you see "+/- <0.01%" it means that there was a change in bundle size, but it is a trivial enough amount that it can be ignored.
024674d
to
7de6cff
Compare
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: 6
Out of diff range and nitpick comments (3)
packages/api/router/location/query.forGoogleMaps.handler.ts (1)
30-40
: Ensure index optimization for database queries.The database query in this function involves filtering and retrieving multiple fields. To ensure efficient query performance, consider verifying that appropriate indexes are set on the
orgLocation
table, particularly on theid
,latitude
, andlongitude
fields. This can significantly reduce the query execution time, especially with a large dataset.packages/api/router/orgPhone/mutation.upsert.handler.ts (1)
67-99
: Optimize the database transaction logic for better performance.The current implementation performs multiple database operations within a transaction. To enhance performance, consider batching similar operations or restructuring the logic to minimize the number of calls, especially in scenarios where multiple updates or checks are performed on similar datasets.
packages/ui/components/data-portal/PhoneDrawer/index.tsx (1)
Line range hint
91-111
: Enhance error handling and user feedback in thePhoneDrawer
component.The component interacts with the API for data fetching and updating. Consider implementing more robust error handling and providing user feedback for API request states (e.g., loading, error) to improve the user experience.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (18)
- packages/api/lib/context.ts (1 hunks)
- packages/api/lib/middleware/permissions.ts (3 hunks)
- packages/api/lib/trpc.ts (1 hunks)
- packages/api/router/location/query.forGoogleMaps.handler.ts (2 hunks)
- packages/api/router/location/query.forGoogleMaps.schema.ts (1 hunks)
- packages/api/router/location/query.forLocationCard.handler.ts (1 hunks)
- packages/api/router/location/query.forLocationCard.schema.ts (1 hunks)
- packages/api/router/orgPhone/index.ts (1 hunks)
- packages/api/router/orgPhone/mutation.upsert.handler.ts (1 hunks)
- packages/api/router/orgPhone/mutation.upsert.schema.ts (1 hunks)
- packages/api/router/orgPhone/schemas.ts (1 hunks)
- packages/api/router/orgWebsite/mutation.upsert.schema.ts (2 hunks)
- packages/api/schemas/idPrefix.ts (1 hunks)
- packages/api/schemas/nestedOps.ts (1 hunks)
- packages/api/types/handler.ts (1 hunks)
- packages/ui/components/core/GoogleMap.tsx (2 hunks)
- packages/ui/components/data-portal/PhoneDrawer/index.tsx (4 hunks)
- packages/ui/components/sections/LocationCard.tsx (5 hunks)
Files skipped from review due to trivial changes (2)
- packages/api/router/location/query.forGoogleMaps.schema.ts
- packages/api/router/orgPhone/schemas.ts
Additional comments not posted (12)
packages/api/router/location/query.forLocationCard.schema.ts (1)
7-7
: The addition of theisEditMode
field with a default value offalse
is well-implemented, enhancing the schema's flexibility for UI rendering based on the mode.packages/api/types/handler.ts (1)
9-9
: The modification toTRPCHandlerParams
enhances type safety by appropriately adjusting the context type based on the scenario. This is a clean and effective use of TypeScript's conditional types.packages/api/router/orgWebsite/mutation.upsert.schema.ts (1)
14-14
: The preprocessing step fororgLocationId
usingemptyStringToNull
is well-implemented, enhancing data integrity by converting empty strings tonull
before further processing.packages/api/router/orgPhone/mutation.upsert.schema.ts (1)
34-34
: The implementation ofZUpsertSchema
using a discriminated union for create and update operations is well-designed, providing clear and robust validation rules for different scenarios.packages/api/lib/context.ts (1)
26-26
: The addition ofgenerateId
and settingskipCache
tofalse
increateContextInner
are well-implemented, enhancing the context's functionality and control over caching behavior.packages/api/lib/trpc.ts (1)
58-58
: The enhancement toimportHandler
with caching and type safety using thesatisfies
keyword is well-implemented, optimizing performance and ensuring type correctness.packages/api/router/location/query.forLocationCard.handler.ts (1)
9-65
: The implementation of theforLocationCard
handler, including the handling ofisEditMode
for conditional data visibility and robust error management withhandleError
, is well-executed and aligns with the system's requirements for dynamic data access based on user permissions.packages/api/router/orgPhone/index.ts (1)
71-76
: Ensure proper authorization checks for theupsert
operation.The
upsert
function usespermissionedProcedure('updatePhone')
which suggests it's using the same permissions as the update operation. Verify that this is appropriate for both create and update scenarios within theupsert
operation. If different permissions are needed for create vs update, consider adjusting the permission handling.packages/ui/components/core/GoogleMap.tsx (1)
35-38
: Validate the integration ofuseEditMode
in the component logic.The
useEditMode
hook is used to conditionally fetch data based on the edit mode. Ensure that this integration does not introduce any side effects or issues with the map's rendering and interaction, especially when toggling between edit modes.packages/api/lib/middleware/permissions.ts (1)
24-26
: Confirm the appropriateness of returningtrue
directly in thecheckPermissions
function.The function now returns
true
directly if the user has 'root' permissions. Ensure that this change aligns with the intended permission logic and does not inadvertently bypass other important checks.Verification successful
The verification process has confirmed that the direct return of
true
in thecheckPermissions
function for users with 'root' permissions is consistent with other parts of the codebase and aligns with the intended permission logic. This approach does not bypass other essential checks, as there are additional checks for specific permissions in the same file. Therefore, the implementation is appropriate and does not introduce any issues.
- File:
packages/api/lib/middleware/permissions.ts
- Direct return for 'root' permissions and additional checks for specific permissions.
- File:
packages/auth/next-auth/session.browser.ts
- Similar implementation of direct return for 'root' permissions.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the direct return of `true` for root permissions in `checkPermissions`. # Test: Check for bypassing of permission checks. Expect: No bypassing of essential checks. rg --type typescript $'permissions.includes'Length of output: 78
Script:
#!/bin/bash # Description: Re-verify the direct return of `true` for root permissions in `checkPermissions`. # Test: Check for bypassing of permission checks. Expect: No bypassing of essential checks. rg 'permissions.includes'Length of output: 330
packages/api/schemas/nestedOps.ts (1)
126-136
: Ensure type safety and correct usage of the enhancedconnectOneRequired
function.The modifications to
connectOneRequired
introduce flexibility in specifying the key for connections. Verify that these changes are correctly integrated throughout the codebase and that they maintain type safety.Verification successful
The
connectOneRequired
function appears to be correctly integrated and used with appropriate type safety across the codebase. The function is utilized in various schema files, and the parameters passed in these usages align with the expected types and functionality described in the function's definition. Here are the specific points of integration:
- User Mutations: The function is used in
mutation.create.schema.ts
andmutation.adminCreate.schema.ts
to handle user types.- Organization Phone Mutations: In
mutation.upsert.handler.ts
, the function is used to connect country data, correctly utilizing the flexibility to specify a key.Overall, the modifications to
connectOneRequired
are well-integrated and maintain type safety as intended.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration and type safety of the enhanced `connectOneRequired` function. # Test: Search for usages of `connectOneRequired` and check for type issues. Expect: Correct types and no integration errors. rg --type typescript $'connectOneRequired'Length of output: 76
Script:
#!/bin/bash # Description: Re-verify the integration and type safety of the enhanced `connectOneRequired` function. # Corrected Test: Search for usages of `connectOneRequired` in TypeScript files and check for type issues. Expect: Correct types and no integration errors. rg 'connectOneRequired' --glob '*.ts' --glob '*.tsx'Length of output: 940
packages/ui/components/sections/LocationCard.tsx (1)
64-69
: Verify the conditional data fetching logic in theLocationCard
component.The component uses conditional logic to fetch data based on the
remoteOnly
andedit
props. Ensure that this logic correctly handles all scenarios and does not lead to unintended fetches or missing data.Verification successful
The conditional data fetching logic in the
LocationCard
component has been verified and appears to be correctly implemented. TheuseQuery
hooks are conditioned on theremoteOnly
andedit
props as follows:
- The query for location data is enabled unless
remoteOnly
is true, ensuring that location data is not fetched when only remote services are needed.- The query for remote services is enabled only when
remoteOnly
is true and an organization ID is present, ensuring that service data is fetched only under appropriate conditions.This setup should effectively prevent unintended data fetches or missing data based on the given conditions.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the conditional data fetching logic in `LocationCard`. # Test: Check for scenarios where data might not be fetched correctly. Expect: Correct fetching in all scenarios. rg --type typescript $'useQuery'Length of output: 66
Script:
#!/bin/bash # Correcting the file type and re-running the search for `useQuery` in TypeScript files to verify the conditional data fetching logic in `LocationCard`. rg --type ts 'useQuery' --files-with-matchesLength of output: 2783
Script:
#!/bin/bash # Extract the relevant code snippet from `LocationCard.tsx` where `useQuery` is used. rg --type ts --context 10 'useQuery' packages/ui/components/sections/LocationCard.tsxLength of output: 1354
Quality Gate passedIssues Measures |
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
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- packages/ui/components/sections/LocationCard.tsx (5 hunks)
- packages/ui/hooks/useGoogleMaps.ts (5 hunks)
- packages/ui/providers/GoogleMaps.tsx (6 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/ui/components/sections/LocationCard.tsx
Additional comments not posted (6)
packages/ui/hooks/useGoogleMaps.ts (3)
19-19
: The addition oftweenGroup
to the return object when the map is ready aligns with the intended UI enhancements.
29-29
: EnsuringtweenGroup
is included in the return object regardless of the map's readiness state maintains consistency and functionality.
55-55
: IncludingtweenGroup
in the interface definitions for both map states enhances clarity and maintains type safety.Also applies to: 64-64
packages/ui/providers/GoogleMaps.tsx (3)
31-31
: UsinguseMemo
to instantiateTweenGroup
follows best practices for performance optimization in React.
74-74
: IncludingtweenGroup
in thecontextValue
object ensures its accessibility throughout the application, facilitating necessary animations.Also applies to: 83-83
113-113
: AddingtweenGroup
to theGoogleMapContextBase
interface enhances type consistency and clarity across the application.
Pull Request type
Please check the type of change your PR introduces:
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
Style
Tests