Organisation model update #2739
Conversation
WalkthroughThis pull request refactors the handling of an organization’s address by replacing a single nested address field with multiple discrete fields. The JSON parsing and serialization logic in the OrgInfo model and its adapter have been updated accordingly, including key renaming. Additionally, user membership data extraction and related GraphQL queries have been refined. UI components and associated tests are modified to directly access the new address fields, and obsolete test files and models have been removed. Changes
Sequence Diagram(s)sequenceDiagram
participant JSON as JSON Data
participant Parser as OrgInfo.fromJson
participant Model as OrgInfo Object
JSON->>Parser: Provide JSON with new address fields
Parser->>Model: Parse fields (city, countryCode, line1, line2, postalCode, state)
Parser->>Model: Extract members and admins from nested JSON
Parser-->>JSON: Return updated OrgInfo Object
sequenceDiagram
participant UI as User Interface
participant Screen as OrgInfoScreen/CustomListTile
participant Data as OrgInfo Object
UI->>Screen: Request organization details
Screen->>Data: Retrieve city & countryCode
alt Both fields available
Data-->>Screen: Provide address details
Screen-->>UI: Display organization location
else Data missing
Screen-->>UI: Skip displaying location
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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
|
Our Pull Request Approval ProcessThanks for contributing! Testing Your CodeRemember, your PRs won't be reviewed until these criteria are met:
Our policies make our code better. ReviewersDo not assign reviewers. Our Queue Monitors will review your PR and assign them.
Reviewing Your CodeYour reviewer(s) will have the following roles:
Other🎯 Please be considerate of our volunteers' time. Contacting the person who assigned the reviewers is not advised unless they ask for your input. Do not @ the person who did the assignment otherwise. |
![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 comments (2)
lib/models/organization/org_info.dart (1)
63-84: 🛠️ Refactor suggestionStandardize JSON field names.
The JSON field names in the API response don't match the model's field names:
- 'avatarURL' vs 'image'
- 'addressLine1' vs 'line1'
- 'addressLine2' vs 'line2'
Consider using consistent field names or adding a mapping layer to handle the differences.
test/views/after_auth_screens/org_info_screen_test.dart (1)
16-64:⚠️ Potential issueUpdate mockOrgInfo to include all required fields.
The mock data has been partially updated to use the new flattened address structure, but:
- The
membersandadminsarrays are commented out, which could affect tests that depend on this data- The
idfield has been renamed from_idto match the new modelPlease verify if the commented-out members and admins data is still needed for the tests. If so, update them to match the new model structure.
🧹 Nitpick comments (4)
lib/models/organization/org_info.dart (1)
35-61: Simplify member extraction logic.The current implementation is overly complex with multiple type casts and transformations.
Consider simplifying the member extraction:
- final membersJson = json['members'] as Map<String, dynamic>?; - final List<dynamic> edgesDynamic = - membersJson?['edges'] as List<dynamic>? ?? []; - - final List<Map<String, dynamic>> memberEdges = - edgesDynamic.map((e) => e as Map<String, dynamic>).toList(); - - final List<User> members = memberEdges - .map( - (e) => - User.fromJson(e['node'] as Map<String, dynamic>, fromOrg: true), - ) - .toList(); + final List<User> members = (((json['members'] as Map<String, dynamic>?)?['edges'] + as List<dynamic>?) ?? []) + .map((e) => User.fromJson( + (e as Map<String, dynamic>)['node'] as Map<String, dynamic>, + fromOrg: true)) + .toList();lib/models/user/user_info.dart (1)
37-42: Simplify organization data extraction.The current implementation uses multiple intermediate variables and type casts.
Consider simplifying:
- final Map<String, dynamic>? org = - userData['organizationsWhereMember'] as Map<String, dynamic>?; - final List<dynamic>? edges = org?['edges'] as List<dynamic>?; - final List<Map<String, dynamic>>? orgList = - edges?.map((e) => e as Map<String, dynamic>).toList(); + final edges = ((userData['organizationsWhereMember'] + as Map<String, dynamic>?)?['edges'] as List<dynamic>?) ?? [];lib/utils/queries.dart (1)
38-38: Consider implementing pagination for large datasets.The fixed limit of 32 records (
first:32) for both organizations and members might not scale well for:
- Organizations with many members
- Users who are members of many organizations
Consider implementing proper pagination using cursor-based pagination with
afterparameter.Also applies to: 88-88
lib/views/after_auth_screens/org_info_screen.dart (1)
124-130: Consider showing more address details.The current implementation only shows city and country code. Consider showing a more comprehensive address by including:
- Address lines (when available)
- Postal code
- State/Province
This would provide users with complete location information.
-if (orgInfo.city != null && orgInfo.countryCode != null) +if (orgInfo.city != null || orgInfo.countryCode != null) Padding( padding: const EdgeInsets.only(top: 8, left: 8), child: Center( child: Text( - '${orgInfo.city}, ${orgInfo.countryCode}', + [ + if (orgInfo.addressLine1 != null) orgInfo.addressLine1, + if (orgInfo.addressLine2 != null) orgInfo.addressLine2, + if (orgInfo.city != null) orgInfo.city, + if (orgInfo.state != null) orgInfo.state, + if (orgInfo.postalCode != null) orgInfo.postalCode, + if (orgInfo.countryCode != null) orgInfo.countryCode, + ].where((e) => e != null).join(', '), style: const TextStyle( fontSize: 16.0, color: Color.fromARGB(255, 179, 168, 168), ), ), ), ),
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
lib/models/organization/org_info.dart(4 hunks)lib/models/organization/org_info.g.dart(2 hunks)lib/models/organization/org_info_address.dart(0 hunks)lib/models/user/user_info.dart(2 hunks)lib/utils/queries.dart(2 hunks)lib/views/after_auth_screens/org_info_screen.dart(1 hunks)lib/widgets/custom_list_tile.dart(1 hunks)test/model_tests/events/event_agenda_item_test.dart(1 hunks)test/model_tests/organization/org_info_address_test.dart(0 hunks)test/model_tests/organization/org_info_test.dart(1 hunks)test/views/after_auth_screens/org_info_screen_test.dart(4 hunks)test/widget_tests/widgets/custom_list_tile_test.dart(1 hunks)
💤 Files with no reviewable changes (2)
- test/model_tests/organization/org_info_address_test.dart
- lib/models/organization/org_info_address.dart
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Checking codebase
🔇 Additional comments (11)
lib/models/organization/org_info.g.dart (2)
28-33: LGTM! Address fields are properly mapped.The generated code correctly maps the new address fields with sequential field IDs.
58-68: LGTM! Write method properly serializes all fields.The write method correctly handles all 14 fields, including the new address components.
test/model_tests/events/event_agenda_item_test.dart (1)
32-32: LGTM! ID field name standardized.The change from '_id' to 'id' aligns with the standardization of identifier field names across the codebase.
lib/models/organization/org_info.dart (1)
149-171: LGTM! Well-documented address fields.The new address fields are properly documented with clear descriptions.
lib/models/user/user_info.dart (1)
56-60: LGTM! Null-safe organization list initialization.The code properly handles null cases by defaulting to an empty list.
lib/widgets/custom_list_tile.dart (1)
119-124: LGTM! The address display logic has been updated correctly.The changes correctly reflect the new organization model structure by directly accessing
cityandcountryCodefields instead of using a nested address object.test/views/after_auth_screens/org_info_screen_test.dart (1)
70-124: LGTM! The mock data structure has been updated correctly.The changes to
mockOrgInfo2correctly reflect the new organization model structure with flattened address fields.test/widget_tests/widgets/custom_list_tile_test.dart (1)
98-99: LGTM! The test data has been updated correctly.The OrgInfo instantiation has been updated to use the new flattened address structure with direct
cityandcountryCodefields.lib/utils/queries.dart (2)
38-60: LGTM! Enhanced organization data retrieval in registration.The addition of detailed organization fields in the registration mutation improves data completeness and aligns with the organization model update.
88-110: LGTM! Consistent organization data retrieval in login.The login query maintains consistency with the registration mutation by including the same detailed organization fields.
lib/views/after_auth_screens/org_info_screen.dart (1)
124-130: LGTM! Improved address field handling.The update properly handles the new organization address structure by:
- Checking both city and countryCode for null values
- Using direct field access instead of the nested address object
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop-postgres #2739 +/- ##
====================================================
- Coverage 96.38% 95.16% -1.22%
====================================================
Files 189 188 -1
Lines 10029 10065 +36
====================================================
- Hits 9666 9578 -88
- Misses 363 487 +124 ☔ View full report in Codecov by Sentry. |
|
|
Currently, certain features such as membership requests, fetching all organizations, and accessing the creator as a regular member of an organization have not yet been implemented in the API or the admin interface. As a result, writing tests related to these features and screen is not feasible at this time. |
|
@MohitMaulekhi I think we have support for fetching orgs in api. Once check there and open issue there if itn't so. Let's merge this PR later on if we need any changes in API |
What kind of change does this PR introduce?
Bug fix, model updates, and query updates for organizations.
Issue Number:
Fixes #2734
Snapshots/Videos:
WhatsApp.Video.2025-02-17.at.9.43.09.AM.mp4
Summary
This pull request addresses an existing problem by fixing bugs and updating models and queries related to organizations. It aims to improve the interaction with organization-related features in the app.
Does this PR introduce a breaking change?
No
Other information
The backend currently lacks the following capabilities:
As discussed on Slack, these features need to be implemented in the app. For now, only users who have joined an organization can interact with organization-related features.
The aforementioned capabilities need to be first handled in the API and then in the admin panel to allow implementation in the app. Some test cases were also commented out as the UI related to membership is present, but due to lack of information on how the JSON will be for membership requests, it is not possible to work on them.
Have you read the contributing guide?
Yes
Summary by CodeRabbit
New Features
Bug Fixes