Skip to content
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

Add group setting options #8

Merged
merged 7 commits into from
Apr 5, 2024
Merged

Add group setting options #8

merged 7 commits into from
Apr 5, 2024

Conversation

cp-amisha-i
Copy link
Collaborator

@cp-amisha-i cp-amisha-i commented Apr 1, 2024

Summary by CodeRabbit

  • New Features
    • Enhanced image management with the ability to update and more efficiently delete images.
    • Improved group settings in the app with new UI components for better user interaction.
    • Added more robust error handling across image management and user/group data operations.
  • Refactor
    • Transitioned to using reactive programming patterns (Combine) for better error handling and asynchronous operations in data management and UI updates.
    • Updated methods for user and group management to return reactive publishers for streamlined error handling and data flow.
  • Bug Fixes
    • Fixed issues in deletion and update operations for images and user data with improved error handling mechanisms.

Copy link

coderabbitai bot commented Apr 1, 2024

Walkthrough

This update introduces significant improvements across various components of the application, focusing on error handling, asynchronous operations with Combine, and UI enhancements. The changes include the addition of methods for better image management, refactoring of repository methods to use AnyPublisher for reactive programming, updates in user store operations for enhanced error handling and deletion capabilities, and a comprehensive overhaul of the GroupSettingView to incorporate dynamic UI elements based on the view state, alongside updates in the view model to support these changes.

Changes

File Path Change Summary
.../Firebase/StorageManager.swift Added updateImage, refactored deleteImage with error handling, changed uploadImage return type
.../Repository/ShareCodeRepository.swift Methods now return AnyPublisher, improved error handling
.../Store/UserStore.swift Updated methods for better error handling, added deleteUser
.../Group/Group Setting/GroupSettingView.swift Added imports, updated UI components, conditional rendering
.../Group/Group Setting/GroupSettingViewModel.swift Added Combine, properties for group management, methods for data fetching and UI actions

🐰✨
A hop, a skip, in the digital glade,
Where code and pixels gently sway.
With every push, and merge, and pull,
We craft a warren, vast and full.
Through fields of data, swift we dart,
With Combine's stream, our art we chart.
So here's to changes, fresh and bright,
In our burrow of code, we find delight.
🌟🐾

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?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a 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 Status

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 66e3525 and f5dfbe8.
Files ignored due to path filters (2)
  • BaseStyle/BaseStyle/Resource/BaseAssets.xcassets/Images/Group.imageset/Group.png is excluded by !**/*.png
  • BaseStyle/BaseStyle/Resource/BaseAssets.xcassets/Images/User.imageset/User.png is excluded by !**/*.png
Files selected for processing (23)
  • BaseStyle/BaseStyle.xcodeproj/project.pbxproj (4 hunks)
  • BaseStyle/BaseStyle/CustomUI/ProfileImageView.swift (1 hunks)
  • BaseStyle/BaseStyle/Resource/AppColors.swift (1 hunks)
  • BaseStyle/BaseStyle/Resource/BaseAssets.xcassets/Colors/DisableLightTextColor.colorset/Contents.json (1 hunks)
  • BaseStyle/BaseStyle/Resource/BaseAssets.xcassets/Images/Contents.json (1 hunks)
  • BaseStyle/BaseStyle/Resource/BaseAssets.xcassets/Images/Group.imageset/Contents.json (1 hunks)
  • BaseStyle/BaseStyle/Resource/BaseAssets.xcassets/Images/User.imageset/Contents.json (1 hunks)
  • Data/Data/Router/AppRoute.swift (3 hunks)
  • Splito.xcodeproj/project.pbxproj (4 hunks)
  • Splito/UI/Home/Expense/Detail Selection/ChooseGroupView.swift (1 hunks)
  • Splito/UI/Home/Expense/Detail Selection/ChoosePayerView.swift (1 hunks)
  • Splito/UI/Home/Groups/Add Member/InviteMemberView.swift (1 hunks)
  • Splito/UI/Home/Groups/Add Member/JoinMemberView.swift (1 hunks)
  • Splito/UI/Home/Groups/Create Group/CreateGroupView.swift (1 hunks)
  • Splito/UI/Home/Groups/Create Group/CreateGroupViewModel.swift (1 hunks)
  • Splito/UI/Home/Groups/Group/Group Setting/GroupSettingView.swift (1 hunks)
  • Splito/UI/Home/Groups/Group/Group Setting/GroupSettingViewModel.swift (1 hunks)
  • Splito/UI/Home/Groups/Group/GroupHomeView.swift (1 hunks)
  • Splito/UI/Home/Groups/Group/GroupHomeViewModel.swift (1 hunks)
  • Splito/UI/Home/Groups/GroupListView.swift (1 hunks)
  • Splito/UI/Home/Groups/GroupRouteView.swift (1 hunks)
  • Splito/UI/User Profile/UserProfileImageView.swift (2 hunks)
  • Splito/UI/User Profile/UserProfileView.swift (1 hunks)
Files skipped from review due to trivial changes (3)
  • BaseStyle/BaseStyle/Resource/BaseAssets.xcassets/Images/Contents.json
  • BaseStyle/BaseStyle/Resource/BaseAssets.xcassets/Images/Group.imageset/Contents.json
  • BaseStyle/BaseStyle/Resource/BaseAssets.xcassets/Images/User.imageset/Contents.json
Additional Context Used
Additional comments not posted (27)
BaseStyle/BaseStyle/Resource/BaseAssets.xcassets/Colors/DisableLightTextColor.colorset/Contents.json (1)

1-20: The color asset DisableLightTextColor is correctly defined within the Display P3 color space and is set to be universally applicable across all device types. This addition aligns with the objectives of enhancing the application's visual resources.

BaseStyle/BaseStyle/Resource/AppColors.swift (1)

21-21: The addition of disableLightText as a public constant is consistent with the project's existing pattern for defining color constants. This change will facilitate the use of this new color asset throughout the application.

Splito/UI/Home/Groups/GroupRouteView.swift (1)

29-30: The addition of the .GroupSettingView case to GroupRouteView is correctly implemented, following the established routing pattern and adhering to best practices. This change supports the new group setting functionalities as outlined in the PR objectives.

Splito/UI/Home/Groups/Group/GroupHomeViewModel.swift (1)

53-54: The addition of the handleSettingButtonTap() function in GroupHomeViewModel is correctly implemented, facilitating navigation to the GroupSettingView with the appropriate groupId. This change enhances the application's navigation capabilities in relation to group settings.

Data/Data/Router/AppRoute.swift (2)

32-32: The addition of the GroupSettingView case to the AppRoute enum is correctly implemented, enhancing the application's routing capabilities to include group settings functionality. This change aligns with the PR objectives.


53-54: The renaming actions within the key computed property of the AppRoute enum improve clarity by differentiating between user profile views and group setting views. These changes are consistent with best practices for naming and enhance the readability of the routing logic.

Also applies to: 74-75

Splito/UI/Home/Groups/Add Member/InviteMemberView.swift (1)

52-52: The addition of a background color to the InviteMemberView enhances its visual presentation and aligns with the application's design theme. This change contributes to the overall UI improvements outlined in the PR objectives.

Splito/UI/Home/Groups/Group/Group Setting/GroupSettingViewModel.swift (1)

9-80: The updates to the GroupSettingViewModel, including the addition of new properties, methods, and the use of the Combine framework, significantly enhance its functionality related to group settings. These changes are well-implemented, align with modern development practices, and adhere to the MVVM pattern.

Splito/UI/Home/Expense/Detail Selection/ChooseGroupView.swift (1)

71-72: The refactoring of the image handling logic in GroupCellView to use GroupProfileImageView simplifies the code and enhances maintainability. This change aligns with the objectives of utilizing new UI components for profile image display and adheres to best practices for component reuse in SwiftUI.

Splito/UI/Home/Groups/Add Member/JoinMemberView.swift (1)

38-38: Adding a background color enhances the visual presentation of JoinMemberView. Ensure that backgroundColor is defined and accessible within this context, and that it aligns with the app's color scheme for consistency.

BaseStyle/BaseStyle/CustomUI/ProfileImageView.swift (2)

11-47: The implementation of MemberProfileImageView uses Kingfisher efficiently to load and display images with a circular clip shape. The use of ResizingImageProcessor for aspect fill and setting a frame size is appropriate for profile images. Ensure that the placeholder image .user and the overlay color Color.gray align with the app's design guidelines.


50-78: GroupProfileImageView follows a similar pattern to MemberProfileImageView, with the distinction of using a rounded rectangle clip shape. This consistency in handling images is good practice. Again, ensure that the placeholder image .group and overlay color are consistent with the app's UI design standards.

Splito/UI/User Profile/UserProfileImageView.swift (2)

12-12: Renaming ProfileImageView to UserProfileImageView clarifies the view's purpose, aligning with the PR's objectives to enhance profile image handling. This change improves code readability and maintainability.


12-12: The implementation of UserProfileImageView effectively handles both scenarios of displaying a profile image from a URL or a locally selected image. The use of KFImage for remote images and SwiftUI's Image for local images, along with the tap gesture for editing, is well-integrated. Ensure that the placeholder image .user and the edit icon .profileEditPencil are correctly defined and accessible.

Splito/UI/Home/Expense/Detail Selection/ChoosePayerView.swift (1)

85-85: Refactoring MemberCellView to use MemberProfileImageView for displaying member images simplifies the code and enhances readability. This change aligns with the PR's objectives to streamline profile image handling. Ensure that MemberProfileImageView is correctly configured to display images as intended in this context.

Splito/UI/Home/Groups/Create Group/CreateGroupViewModel.swift (1)

34-34: Replacing selectedGroupType with group and adding isOpenForEdit in CreateGroupViewModel enhances the model's flexibility for group creation and editing. Ensure that these properties are used consistently throughout the view model to manage group details and editing state effectively.

Splito/UI/Home/Groups/Create Group/CreateGroupView.swift (1)

118-123: > 📝 NOTE

This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-1]

Removing GroupTypeSelectionView simplifies the CreateGroupView, focusing on essential elements for group creation. This change aligns with the PR's objectives to enhance the UI and improve maintainability. Ensure that the removal does not affect the user's ability to specify group types if that functionality is still required.

Splito/UI/Home/Groups/Group/GroupHomeView.swift (1)

30-41: Adding a toolbar with a settings button in GroupHomeView enhances the user experience by providing easy access to group settings. Ensure that the handleSettingButtonTap method is implemented correctly to navigate to the settings view and that the gear icon aligns with the app's design guidelines.

Splito/UI/Home/Groups/GroupListView.swift (1)

78-78: Refactoring GroupListCellView to use GroupProfileImageView for displaying group images simplifies the code and enhances readability. This change aligns with the PR's objectives to streamline profile image handling. Ensure that GroupProfileImageView is correctly configured to display images as intended in this context.

Splito/UI/Home/Groups/Group/Group Setting/GroupSettingView.swift (5)

8-11: New imports for Data, BaseStyle, and Kingfisher have been added. Ensure that these libraries are used effectively within the file and that there are no unnecessary imports.


18-46: The conditional rendering logic in GroupSettingView is clear and follows a standard pattern for handling loading, success, and potentially error states (though error handling isn't explicitly shown here). It's good practice to handle different view states to enhance user experience.


49-68: GroupTitleView effectively uses SwiftUI's HStack for layout, ensuring that the group's name and an "Edit" button are displayed. The use of .padding(.horizontal, 22) provides consistent spacing. Consider making the "Edit" text a button that triggers an action if it's not purely decorative.


71-104: GroupMembersView provides a clear and interactive UI for displaying group members and adding new ones. The use of LazyVStack for member listing is appropriate for performance. Ensure that the "Add people to group" action is properly connected and tested, as it's crucial for user interaction.


107-152: GroupMemberCellView effectively displays member details with a clear layout. The initialization logic that differentiates between the current user and other members is a nice touch. Ensure that the "settled up" text is dynamically updated based on the member's status, as it currently appears static.

Splito/UI/User Profile/UserProfileView.swift (1)

25-25: Renaming ProfileImageView to UserProfileImageView in UserProfileView aligns with the intent to make the naming more specific and clear. This change improves code readability and maintainability by clearly distinguishing between user profile images and other types of profile images that might exist in the application.

Splito.xcodeproj/project.pbxproj (2)

166-166: The renaming of ProfileImageView to UserProfileImageView is correctly reflected in the file reference. This change aligns with the PR objectives to enhance profile image handling.


490-490: The inclusion of UserProfileImageView.swift in the project's sources build phase is correctly updated, ensuring that the renamed file is compiled as part of the build process.

Copy link

@coderabbitai coderabbitai bot left a 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 Status

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between f5dfbe8 and 0bbc456.
Files selected for processing (12)
  • Data/Data/Helper/Firebase/StorageManager.swift (1 hunks)
  • Data/Data/Model/Groups.swift (1 hunks)
  • Data/Data/Repository/GroupRepository.swift (3 hunks)
  • Data/Data/Router/AppRoute.swift (3 hunks)
  • Data/Data/Store/GroupStore.swift (1 hunks)
  • Splito/UI/Home/Groups/Create Group/CreateGroupView.swift (5 hunks)
  • Splito/UI/Home/Groups/Create Group/CreateGroupViewModel.swift (4 hunks)
  • Splito/UI/Home/Groups/Group/Group Setting/GroupSettingView.swift (1 hunks)
  • Splito/UI/Home/Groups/Group/Group Setting/GroupSettingViewModel.swift (1 hunks)
  • Splito/UI/Home/Groups/Group/GroupHomeViewModel.swift (1 hunks)
  • Splito/UI/Home/Groups/GroupListViewModel.swift (1 hunks)
  • Splito/UI/Home/Groups/GroupRouteView.swift (1 hunks)
Files skipped from review as they are similar to previous changes (6)
  • Data/Data/Router/AppRoute.swift
  • Splito/UI/Home/Groups/Create Group/CreateGroupViewModel.swift
  • Splito/UI/Home/Groups/Group/Group Setting/GroupSettingView.swift
  • Splito/UI/Home/Groups/Group/Group Setting/GroupSettingViewModel.swift
  • Splito/UI/Home/Groups/Group/GroupHomeViewModel.swift
  • Splito/UI/Home/Groups/GroupRouteView.swift
Additional comments not posted (9)
Data/Data/Model/Groups.swift (1)

10-10: The addition of Hashable conformance to the Groups struct is a positive change, enhancing its utility in collections and SwiftUI views. Ensure that all uses of Groups instances throughout the application are reviewed to accommodate this new conformance.

Splito/UI/Home/Groups/GroupListViewModel.swift (1)

51-51: Explicitly passing group: nil when navigating to .CreateGroupView clarifies the intention to create a new group, enhancing code readability and maintainability.

Data/Data/Helper/Firebase/StorageManager.swift (2)

54-58: The updateImage method is a valuable addition, facilitating the update of images with proper error handling. Consider adding documentation comments to explain its purpose and usage.


62-71: Refactoring deleteImage to include a completion handler for error handling enhances the method's utility and robustness. Good job on improving error handling.

Data/Data/Store/GroupStore.swift (1)

29-37: Modifying updateGroup to return the docID of the updated group is a significant improvement, enhancing feedback and control in group management. Consider adding documentation to clarify expected errors and their handling.

Splito/UI/Home/Groups/Create Group/CreateGroupView.swift (2)

24-25: Adding the imageUrl property to AddGroupNameView enhances the dynamic UI capabilities. Ensure that a fallback image or placeholder is provided in case the KFImage fails to load.


36-36: Updating the navigation bar title based on the presence of a group (creating vs. editing) is a good UX improvement. It provides clarity to the user about the current context.

Data/Data/Repository/GroupRepository.swift (2)

72-74: Modifying addMemberToGroup to return a String enhances feedback and control in member management. Good alignment with changes in GroupStore.swift.


140-163: The addition of updateGroupWithImage is a significant improvement, incorporating robust image management into group updates. Consider adding documentation to explain the workflow and error handling of this function.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Review Status

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 0bbc456 and 15ba9ce.
Files selected for processing (6)
  • Data/Data/Repository/GroupRepository.swift (2 hunks)
  • Data/Data/Router/RouterView.swift (1 hunks)
  • Data/Data/Store/GroupStore.swift (1 hunks)
  • Splito/UI/Home/Groups/Create Group/CreateGroupView.swift (5 hunks)
  • Splito/UI/Home/Groups/Group/Group Setting/GroupSettingView.swift (1 hunks)
  • Splito/UI/Home/Groups/Group/Group Setting/GroupSettingViewModel.swift (1 hunks)
Files skipped from review as they are similar to previous changes (3)
  • Data/Data/Store/GroupStore.swift
  • Splito/UI/Home/Groups/Create Group/CreateGroupView.swift
  • Splito/UI/Home/Groups/Group/Group Setting/GroupSettingViewModel.swift

Data/Data/Router/RouterView.swift Show resolved Hide resolved
Data/Data/Repository/GroupRepository.swift Outdated Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a 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 Status

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 15ba9ce and 208a3db.
Files selected for processing (3)
  • Data/Data/Repository/GroupRepository.swift (2 hunks)
  • Splito/UI/Home/Groups/Group/Group Setting/GroupSettingView.swift (1 hunks)
  • Splito/UI/Home/Groups/Group/Group Setting/GroupSettingViewModel.swift (1 hunks)
Files skipped from review as they are similar to previous changes (3)
  • Data/Data/Repository/GroupRepository.swift
  • Splito/UI/Home/Groups/Group/Group Setting/GroupSettingView.swift
  • Splito/UI/Home/Groups/Group/Group Setting/GroupSettingViewModel.swift

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 13

Review Status

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 208a3db and f5310d8.
Files selected for processing (16)
  • Data/Data/Helper/Firebase/StorageManager.swift (2 hunks)
  • Data/Data/Repository/ExpenseRepository.swift (1 hunks)
  • Data/Data/Repository/GroupRepository.swift (4 hunks)
  • Data/Data/Repository/ShareCodeRepository.swift (2 hunks)
  • Data/Data/Repository/UserRepository.swift (3 hunks)
  • Data/Data/Services/ServiceError.swift (3 hunks)
  • Data/Data/Store/ExpenseStore.swift (1 hunks)
  • Data/Data/Store/GroupStore.swift (4 hunks)
  • Data/Data/Store/ShareCodeStore.swift (2 hunks)
  • Data/Data/Store/UserStore.swift (4 hunks)
  • Splito/UI/Home/Expense/AddExpenseViewModel.swift (1 hunks)
  • Splito/UI/Home/Groups/Add Member/InviteMemberViewModel.swift (2 hunks)
  • Splito/UI/Home/Groups/Create Group/CreateGroupView.swift (5 hunks)
  • Splito/UI/Home/Groups/Create Group/CreateGroupViewModel.swift (5 hunks)
  • Splito/UI/Home/Groups/Group/Group Setting/GroupSettingView.swift (1 hunks)
  • Splito/UI/Home/Groups/Group/Group Setting/GroupSettingViewModel.swift (1 hunks)
Files skipped from review as they are similar to previous changes (5)
  • Data/Data/Repository/GroupRepository.swift
  • Splito/UI/Home/Groups/Create Group/CreateGroupView.swift
  • Splito/UI/Home/Groups/Create Group/CreateGroupViewModel.swift
  • Splito/UI/Home/Groups/Group/Group Setting/GroupSettingView.swift
  • Splito/UI/Home/Groups/Group/Group Setting/GroupSettingViewModel.swift
Additional comments not posted (8)
Data/Data/Services/ServiceError.swift (1)

21-21: The addition of the alreadyExists case to the ServiceError enum is appropriate for handling data duplication errors.

Splito/UI/Home/Groups/Add Member/InviteMemberViewModel.swift (3)

36-45: The refactoring of generateInviteCode to use Combine's sink method for handling asynchronous responses is correctly implemented.


51-51: The refactoring of fetchGroup to use Combine's sink method for handling asynchronous responses is correctly implemented.


62-69: The refactoring of storeSharedCode to use Combine's sink method for handling asynchronous responses is correctly implemented.

Splito/UI/Home/Expense/AddExpenseViewModel.swift (1)

64-72: The refactoring of saveExpense to use Combine's sink method for handling the asynchronous response from addExpense is correctly implemented, including error handling and state management.

Data/Data/Store/ShareCodeStore.swift (1)

17-32: The refactoring of addSharedCode to use a Future and return an AnyPublisher<Void, ServiceError>, including error handling and logging adjustments, is correctly implemented.

Data/Data/Store/ExpenseStore.swift (1)

17-32: The refactoring of addExpense to use a Future and return an AnyPublisher<Void, ServiceError>, including error handling and success case adjustments, is correctly implemented.

Data/Data/Store/UserStore.swift (1)

58-70: > 📝 NOTE

This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [52-67]

Improve error handling in fetchUsers by distinguishing between different error types.

  func fetchUsers() -> AnyPublisher<[AppUser], ServiceError> {
      Future { [weak self] promise in
          guard let self else {
              promise(.failure(.unexpectedError))
              return
          }
          self.database.collection(self.DATABASE_NAME).getDocuments { snapshot, error in
              if let error {
                  LogE("UserStore :: \(#function) error: \(error.localizedDescription)")
-                 promise(.failure(.databaseError))
+                 promise(.failure(.networkError))
                  return
              }
              guard let snapshot else {
                  LogE("UserStore :: \(#function) The document is not available.")
                  promise(.failure(.dataNotFound))
                  return
              }
              do {
                  let users = try snapshot.documents.compactMap { document in
                      try document.data(as: AppUser.self)
                  }
                  promise(.success(users))
              } catch {
                  LogE("UserStore :: \(#function) Decode error: \(error.localizedDescription)")
                  promise(.failure(.decodingError))
              }
          }
      }.eraseToAnyPublisher()
  }

Distinguishing between different error types, such as networkError and decodingError, provides more detailed feedback to the caller, enabling better error handling and user experience.

Data/Data/Helper/Firebase/StorageManager.swift Outdated Show resolved Hide resolved
Data/Data/Helper/Firebase/StorageManager.swift Outdated Show resolved Hide resolved
Data/Data/Helper/Firebase/StorageManager.swift Outdated Show resolved Hide resolved
Data/Data/Store/UserStore.swift Outdated Show resolved Hide resolved
Data/Data/Store/UserStore.swift Show resolved Hide resolved
Data/Data/Repository/ExpenseRepository.swift Show resolved Hide resolved
Data/Data/Repository/ShareCodeRepository.swift Outdated Show resolved Hide resolved
Data/Data/Repository/UserRepository.swift Show resolved Hide resolved
Data/Data/Repository/UserRepository.swift Show resolved Hide resolved
Copy link

@coderabbitai coderabbitai bot left a 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

Review Status

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between f5310d8 and 9f2c972.
Files selected for processing (5)
  • Data/Data/Helper/Firebase/StorageManager.swift (2 hunks)
  • Data/Data/Repository/ShareCodeRepository.swift (2 hunks)
  • Data/Data/Store/UserStore.swift (4 hunks)
  • Splito/UI/Home/Groups/Group/Group Setting/GroupSettingView.swift (1 hunks)
  • Splito/UI/Home/Groups/Group/Group Setting/GroupSettingViewModel.swift (1 hunks)
Files skipped from review as they are similar to previous changes (3)
  • Data/Data/Repository/ShareCodeRepository.swift
  • Data/Data/Store/UserStore.swift
  • Splito/UI/Home/Groups/Group/Group Setting/GroupSettingView.swift
Additional comments not posted (10)
Data/Data/Helper/Firebase/StorageManager.swift (3)

29-59: Consider adding documentation comments for the uploadImage method to explain its purpose, parameters, return type, and any side effects, such as the possibility of cancellation.


62-69: In the updateImage method, ensure that errors from deleteImage are properly handled. Currently, it catches errors but does not differentiate between them. Consider adding specific error handling or logging for different types of errors.


72-85: The deleteImage method's error handling can be improved by providing more specific error messages based on the type of error encountered. This would aid in debugging and provide clearer feedback to the user or calling function.

Splito/UI/Home/Groups/Group/Group Setting/GroupSettingViewModel.swift (7)

9-10: Ensure that the imports are organized and only necessary modules are imported to keep the code clean and improve compile-time.


14-26: Consider documenting the properties of GroupSettingViewModel to explain their roles within the view model. This enhances code readability and maintainability.


39-51: Add error handling for the fetchGroupDetails method to ensure that the UI can respond appropriately to different error scenarios. This could involve updating the ViewState or displaying an error message.


53-65: For the fetchGroupMembers method, consider adding a fallback or retry mechanism in case of failure to fetch members, to improve the robustness of the application.


113-131: In removeMemberFromGroup, ensure that the UI is updated on the main thread, especially when modifying view-related properties like currentViewState or navigating.


143-154: For deleteGroupWithMembers, similar to removeMemberFromGroup, ensure UI updates are performed on the main thread.


172-176: The ViewState enum is a good addition for managing UI state. Consider documenting each case to explain when they should be used.

@cp-amisha-i cp-amisha-i merged commit e6bfe7f into main Apr 5, 2024
1 check passed
@cp-amisha-i cp-amisha-i deleted the group-setting branch April 5, 2024 11:56
Copy link

@coderabbitai coderabbitai bot left a 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

Review Status

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 9f2c972 and e1f7afb.
Files selected for processing (1)
  • Splito/UI/Home/Groups/Group/Group Setting/GroupSettingViewModel.swift (1 hunks)
Additional comments not posted (4)
Splito/UI/Home/Groups/Group/Group Setting/GroupSettingViewModel.swift (4)

17-18: Ensure that groupId and router are validated or handled appropriately if they are expected to be non-nil throughout the lifecycle of GroupSettingViewModel.


24-26: Validate the presence of group and members in methods where their values are critical to avoid potential runtime errors.


67-70: The fix for validating the presence of userId and group before setting isAdmin has been correctly applied. Ensure that similar checks are applied consistently throughout the code where applicable.


82-95: Refactor the alert creation logic in handleMemberTap to reduce duplication and improve maintainability. The suggested change to create a separate method for alert setup has been correctly implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant