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

Fix UserID and JobID Not Refreshing #287

Merged
merged 3 commits into from
Feb 4, 2025
Merged

Conversation

tobitech
Copy link
Contributor

Story: https://app.shortcut.com/smileid/story/xxx

Summary

  • Fix user id and job id not refreshing on demo app
  • bump build number

Known Issues

N/A.

Test Instructions

N/A.

Screenshot

N/A

@tobitech tobitech self-assigned this Jan 31, 2025
@tobitech tobitech requested a review from a team as a code owner January 31, 2025 16:02
@prfectionist
Copy link

prfectionist bot commented Jan 31, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Code Logic
Verify that calling viewModel.onProductClicked() before setting selectedProduct is the intended order of operations

Copy link

github-actions bot commented Jan 31, 2025

Warnings
⚠️ The source files were changed, but the tests remain unmodified. Consider updating or adding to the tests to match the source changes.

Generated by 🚫 Danger Swift against 2e37b93

@prfectionist
Copy link

prfectionist bot commented Jan 31, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Score
Possible bug
Maintain consistency in version numbers across all build configurations to prevent deployment issues

Ensure CURRENT_PROJECT_VERSION is consistent across all build configurations.
Currently it's set to 44 in some places but remains 43 in others.

Example/SmileID.xcodeproj/project.pbxproj [921-923]

 CODE_SIGN_STYLE = Manual;
-CURRENT_PROJECT_VERSION = 43;
+CURRENT_PROJECT_VERSION = 44;
 DEBUG_INFORMATION_FORMAT = "dwarf-with-dsym";
Suggestion importance[1-10]: 9

Why: This is a critical suggestion as inconsistent version numbers across build configurations can lead to serious deployment and versioning issues. The PR shows this inconsistency needs to be fixed.

9
Possible issue
Reorder state updates to ensure consistent data flow and prevent potential race conditions

Consider moving the onProductClicked() call after setting the selectedProduct to
ensure the view model is updated with the latest selection state.

Example/SmileID/Home/HomeView.swift [26-29]

 Button {
+    selectedProduct = product
     viewModel.onProductClicked()
-    selectedProduct = product
 } label: {
Suggestion importance[1-10]: 7

Why: The suggestion correctly identifies a potential issue where the view model should be updated after the state change to ensure consistent data flow and prevent any race conditions.

7

Copy link
Member

@jumaallan jumaallan left a comment

Choose a reason for hiding this comment

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

Tested and it works for me - I was able to replicate it on main, then tested on your branch and it works well

@jumaallan jumaallan merged commit e125d40 into main Feb 4, 2025
3 checks passed
@jumaallan jumaallan deleted the fix-reported-demo-app-bugs branch February 4, 2025 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants