feat: address visibility settings #1305
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
github-actions
bot
added
📦 db
📦 api
📦 ui
📦 app
automerge
JoeKarow
removed
📦 app
📦 db
📦 api
📦 ui
kodiak: merge.method = 'squash'
github-actions
bot
added
kodiak: merge.method = 'squash'
WalkthroughThis update brings widespread changes across several modules in the project, focusing on enhancing the address handling system. Key alterations include renaming the Changes
The changes enhance address visibility control across the application, ensuring consistent display and handling of address information via updated schemas and UI components. 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 (
|
|
#713 Bundle Size — 3.54MiB (+3.77%).Warning Bundle contains 6 duplicate packages – View duplicate packages Warning Bundle introduced 6 new packages: prisma-query-log, @prisma/client, slugify and 3 more – View changed packages Bundle metrics
|
| Current #713 |
Baseline #704 |
|
|---|---|---|
 Initial JS |
3.14MiB(+4.25%) |
3.01MiB |
 Initial CSS |
9.54KiB |
9.54KiB |
 Cache Invalidation |
39.94% |
54.45% |
| Chunks |
70(+4.48%) |
67 |
| Assets |
83(+3.75%) |
80 |
| Modules |
2043(+1.79%) |
2007 |
 Duplicate Modules |
267(-1.11%) |
270 |
| Duplicate Code |
7.4%(-5.49%) |
7.83% |
| Packages |
217(+2.84%) |
211 |
| Duplicate Packages |
6 |
6 |
Bundle size by type 1 change 1 regression
| Current #713 |
Baseline #704 |
|
|---|---|---|
| JS |
3.42MiB (+3.9%) |
3.29MiB |
| Fonts |
94.54KiB |
94.54KiB |
| Other |
9.58KiB |
9.58KiB |
| CSS |
9.54KiB |
9.54KiB |
| IMG |
8.57KiB |
8.57KiB |
Bundle analysis report Branch IN-984-re-require-city Project dashboard
📦 Next.js Bundle Analysis for @weareinreach/appThis analysis was generated by the Next.js Bundle Analysis action. 🤖 Two Pages Changed SizeThe following pages changed size from the code in this PR compared to its base branch:
DetailsOnly 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 Any third party scripts you have added directly to your app using the 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (2)
packages/db/client/index.ts (1)
Line range hint
55-55: Potential issue with variable redeclaration.The static analysis tool has flagged a potential redeclaration of the
prismavariable. It's important to ensure that this does not cause any unexpected behavior or conflicts in the global scope, especially in development environments where conditional assignments are used.packages/ui/components/data-portal/AddressDrawer.tsx (1)
Line range hint
84-95: Schema updates align well with PR objectives.The changes to make
citya required field and introduceaddressVisibilityare consistent with the PR's goals to enhance visibility settings. However, consider usingz.string().nonempty()forcityto ensure that it is not only present but also non-empty.- city: z.string(), + city: z.string().nonempty(),
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (27)
- InReach.code-workspace (1 hunks)
- apps/app/src/types/nextjs-routes.d.ts (1 hunks)
- packages/api/router/location/lib.formatAddressVisibility.ts (1 hunks)
- packages/api/router/location/mutation.create.schema.ts (2 hunks)
- packages/api/router/location/mutation.update.schema.ts (3 hunks)
- packages/api/router/location/query.forGoogleMaps.handler.ts (2 hunks)
- packages/api/router/location/query.forLocationCard.handler.ts (3 hunks)
- packages/api/router/location/query.forLocationPage.handler.ts (2 hunks)
- packages/api/router/location/query.forLocationPageEdits.handler.ts (1 hunks)
- packages/api/router/location/query.forVisitCard.handler.ts (4 hunks)
- packages/api/router/location/query.getAddress.handler.ts (1 hunks)
- packages/api/router/organization/query.forOrgPage.handler.ts (1 hunks)
- packages/api/router/organization/query.forOrgPageEdits.handler.ts (1 hunks)
- packages/db/client/index.ts (1 hunks)
- packages/db/prisma/data-migrations/index.ts (1 hunks)
- packages/db/prisma/migrations/20240614194612_address_visibility/migration.sql (1 hunks)
- packages/db/prisma/schema.prisma (2 hunks)
- packages/ui/components/core/Toolbar.stories.tsx (1 hunks)
- packages/ui/components/data-portal/AddressDrawer.tsx (4 hunks)
- packages/ui/components/data-portal/LocationDrawer/index.tsx (5 hunks)
- packages/ui/components/sections/ListingBasicInfo.tsx (1 hunks)
- packages/ui/components/sections/LocationCard.tsx (2 hunks)
- packages/ui/mockData/json/location.forLocationCard.json (1 hunks)
- packages/ui/mockData/location.ts (1 hunks)
- packages/ui/mockData/service.ts (1 hunks)
- packages/ui/modals/MoreFilter.stories.tsx (2 hunks)
- packages/ui/modals/MoreFilter.tsx (1 hunks)
Files skipped from review due to trivial changes (6)
- InReach.code-workspace
- apps/app/src/types/nextjs-routes.d.ts
- packages/api/router/location/query.getAddress.handler.ts
- packages/ui/components/core/Toolbar.stories.tsx
- packages/ui/mockData/json/location.forLocationCard.json
- packages/ui/mockData/location.ts
Additional context used
Biome
packages/api/router/location/lib.formatAddressVisibility.ts
[error] 22-22: Useless case clause. (lint/complexity/noUselessSwitchCase)
because the default clause is present:
Unsafe fix: Remove the useless case.
packages/db/client/index.ts
[error] 55-55: Shouldn't redeclare 'prisma'. Consider to delete it or rename it. (lint/suspicious/noRedeclare)
'prisma' is defined here:
packages/ui/components/data-portal/LocationDrawer/index.tsx
[error] 60-62: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
packages/ui/components/sections/ListingBasicInfo.tsx
[error] 134-141: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)
Unsafe fix: Omit the else clause.
Additional comments not posted (24)
packages/db/prisma/data-migrations/index.ts (1)
Line range hint
1-6: Verify the new data migration exports.The newly added exports for data migration scripts should be verified to ensure they are correctly implemented and match the intended functionality changes described in the PR.
Verification successful
Verify the new data migration exports.
The newly added export statements in
packages/db/prisma/data-migrations/index.tshave been verified. All the referenced files exist and their initial lines indicate that they are correctly implemented.
packages/db/prisma/data-migrations/2024-03-21_attribute-supplement-schemas.tspackages/db/prisma/data-migrations/2024-04-03_access-instruction-schemas.tspackages/db/prisma/data-migrations/2024-04-24_update-crowdin-ids.tspackages/db/prisma/data-migrations/2024-04-25_translation-activation-flag.tspackages/db/prisma/data-migrations/2024-06-14_update-baltimore-to-reflect-city-and-county-options.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the new data migration exports are correctly implemented. # Test: List all export statements and manually verify if they match the intended functionality changes. cat packages/db/prisma/data-migrations/index.ts | grep 'export'Length of output: 363
Script:
#!/bin/bash # Verify the existence of each exported file files=( "packages/db/prisma/data-migrations/2024-03-21_attribute-supplement-schemas.ts" "packages/db/prisma/data-migrations/2024-04-03_access-instruction-schemas.ts" "packages/db/prisma/data-migrations/2024-04-24_update-crowdin-ids.ts" "packages/db/prisma/data-migrations/2024-04-25_translation-activation-flag.ts" "packages/db/prisma/data-migrations/2024-06-14_update-baltimore-to-reflect-city-and-county-options.ts" ) for file in "${files[@]}"; do if [[ -f "$file" ]]; then echo "$file exists." else echo "$file is missing." fi doneLength of output: 1939
Script:
#!/bin/bash # Display the first few lines of each exported file to verify their contents files=( "packages/db/prisma/data-migrations/2024-03-21_attribute-supplement-schemas.ts" "packages/db/prisma/data-migrations/2024-04-03_access-instruction-schemas.ts" "packages/db/prisma/data-migrations/2024-04-24_update-crowdin-ids.ts" "packages/db/prisma/data-migrations/2024-04-25_translation-activation-flag.ts" "packages/db/prisma/data-migrations/2024-06-14_update-baltimore-to-reflect-city-and-county-options.ts" ) for file in "${files[@]}"; do echo "Contents of $file:" head -n 10 "$file" echo "-----------------------------------" doneLength of output: 4148
packages/db/prisma/migrations/20240614194612_address_visibility/migration.sql (1)
1-29: Review SQL migration script for address visibility changes.The SQL script properly handles the migration for the new
AddressVisibilityenum and updates theOrgLocationtable accordingly. However, ensure that the script is tested on a backup of the production database to catch any potential issues with existing data, especially concerning thecitycolumn's requirement change.packages/api/router/location/mutation.create.schema.ts (1)
Line range hint
1-24: Review the Zod schema for creating a location entity.The Zod schema correctly enforces the new
addressVisibilityfield usingPrismaEnums. This aligns with the PR's objectives to handle address visibility settings. Ensure that all dependent systems are updated to handle these schema changes.packages/api/router/location/query.forLocationPage.handler.ts (1)
Line range hint
1-37: Review the API handler for the location page.The handler function correctly integrates the new
addressVisibilityfield and utilizes theformatAddressVisibilityfunction to format the address data. This change aligns with the PR's objectives to enhance address visibility handling.packages/api/router/organization/query.forOrgPageEdits.handler.ts (1)
Line range hint
1-34: Review the API handler for organization page edits.The handler function correctly integrates the new
addressVisibilityfield and ensures it is included in the data fetched for organization page edits. This change aligns with the PR's objectives to enhance address visibility handling.packages/ui/modals/MoreFilter.stories.tsx (1)
8-8: Ensure consistent export ofMoreFiltercomponent.The code change reflects a change in how
MoreFilteris exported and used, which aligns with the alterations mentioned in the summary. Ensure that this new export pattern is consistently applied across the project, especially in places whereMoreFilteris imported.packages/api/router/organization/query.forOrgPage.handler.ts (1)
39-39: Approved addition ofaddressVisibilityin API handler.The inclusion of
addressVisibilityin the select query aligns with the database schema changes and the PR's objectives to handle visibility settings. This change is necessary to ensure the frontend receives the correct data based on the new visibility rules.packages/api/router/location/query.forVisitCard.handler.ts (1)
6-6: Integration offormatAddressVisibilityand updates to address visibility handling.The changes here integrate the new
formatAddressVisibilityfunction and update the visibility filtering logic. This is crucial for maintaining consistency in how address visibility is handled across different parts of the application.Ensure that the
formatAddressVisibilityfunction is thoroughly tested to handle all expected input variations effectively.Also applies to: 15-15, 32-32, 41-41
packages/ui/mockData/service.ts (1)
9-9: Ensure correct typing for API output.The casting of
dataasApiOutput['service']['forServiceInfoCard']is a good practice to ensure type safety. This change helps maintain consistency in how data types are handled and ensures that the components consuming this data can rely on its structure.packages/db/client/index.ts (1)
62-62: Ensure proper export of PrismaEnums for address visibility.The addition of
PrismaEnumsexport is crucial for handling the newaddressVisibilityfield consistently across the application. This change appears to be correctly implemented.packages/api/router/location/query.forLocationCard.handler.ts (2)
6-6: Good integration of address formatting utility.Importing
formatAddressVisibilityis essential for applying the new address visibility rules within the location card context. This change supports the feature's requirements effectively.
53-53: Proper application of address visibility formatting.The application of
formatAddressVisibilityto the query result ensures that the address data is correctly transformed according to the visibility settings before being sent to the client.packages/api/router/location/query.forGoogleMaps.handler.ts (1)
33-41: Enhanced query logic for address visibility in maps.The modification to include
addressVisibilityin the query conditions is crucial for ensuring that only appropriately visible locations are processed for map displays. This change aligns well with the privacy and visibility requirements of the feature.packages/api/router/location/mutation.update.schema.ts (2)
3-3: Updated import to support address visibility in update operations.Importing
PrismaEnumsis necessary for defining theaddressVisibilityfield in the update schema. This change is correctly implemented to support the feature.
33-33: Enhanced schema logic for address visibility in updates.The inclusion of
addressVisibilityin the update schema is crucial for ensuring that updates to location data adhere to the new visibility rules. This change aligns well with the privacy and visibility requirements of the feature.packages/api/router/location/query.forLocationPageEdits.handler.ts (1)
27-27: Correct integration of address visibility in edit page queries.The inclusion of
addressVisibilityin the query conditions is crucial for ensuring that the edit page for locations respects the new visibility rules. This change aligns well with the privacy and visibility requirements of the feature.packages/ui/components/data-portal/LocationDrawer/index.tsx (2)
64-68: Add a clarifying comment for address visibility options.It would be beneficial to include a comment explaining the purpose of the
addressVisibilityOptionsarray, especially for future maintainability and understanding of how these options are used within the UI.+ // Options for user to select how their address is displayed on the platform const addressVisibilityOptions: Record<'value' | 'label', string>[] = [ { value: 'FULL', label: 'Show full address' }, { value: 'PARTIAL', label: 'Show city & state/province' }, { value: 'HIDDEN', label: 'Hide address' }, ]
94-99: Ensure proper handling of theSelectcomponent for address visibility.The integration of the
Selectcomponent for address visibility is correctly implemented. It uses theaddressVisibilityOptionsfor user selection, which aligns with the PR's objectives to manage address visibility settings. Ensure that theSelectcomponent's behavior is thoroughly tested, especially its interaction with the form's state and the backend.packages/ui/components/sections/ListingBasicInfo.tsx (1)
30-33: Address visibility logic correctly implemented in UI display.The conditional rendering based on the
addressVisibilityproperty is implemented correctly. This checks if the address should be hidden and, if not, displays it. This change is consistent with the new address visibility feature introduced in the PR.packages/ui/components/sections/LocationCard.tsx (1)
324-326: Correct implementation of conditional rendering based on address visibility.The logic to conditionally render address details based on the
addressVisibilityfield is implemented correctly. This ensures that the address is displayed only when it is not set to 'HIDDEN', which is a crucial part of the feature being added in this PR.[APROVED]
packages/ui/modals/MoreFilter.tsx (1)
518-518: Interface definition forMoreFilterPropsis clear and concise.The
MoreFilterPropsinterface is well-defined, providing clear type definitions for props used in theMoreFiltercomponent. This ensures type safety and improves the maintainability of the code.packages/ui/components/data-portal/AddressDrawer.tsx (2)
163-167: Good addition ofaddressVisibilityOptionsfor UI consistency.This change introduces a clear and user-friendly way to handle address visibility settings in the UI, aligning with the feature's requirements.
471-475: Proper integration ofaddressVisibilityselection in the form.Adding a
Selectcomponent foraddressVisibilitywith a default value of 'FULL' is a good practice, ensuring that there's always a valid state for this field.packages/db/prisma/schema.prisma (1)
691-695: Approval of the AddressVisibility enum definitionThe new
AddressVisibilityenum with valuesFULL,PARTIAL, andHIDDENis well-defined. This enum provides clear options for setting the visibility of addresses, which can be utilized across different parts of the application to conditionally display address information based on this setting.
- Usage: Ensure that the application logic that consumes this enum handles all three cases correctly, especially in UI components where address visibility is critical.
- Integration: Check integration points where this enum replaces previous boolean flags or other visibility indicators to ensure seamless transition and correct behavior across the application.
The definition and default usage of the
AddressVisibilityenum are appropriate and follow best practices for handling conditional visibility in a scalable and maintainable way.
Pull Request type
Please check the type of change your PR introduces:
What is the current behavior?
Issue Number: IN-984
What is the new behavior?
Does this introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
addressVisibilityproperty.Bug Fixes
Refactor
notVisitablewithaddressVisibilityacross multiple components and handlers.LocationDrawercomponent.Chores
_queriesdirectory in workspace settings.