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

[ABW-3776] Unable to create new persona fix #1315

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

danvleju-rdx
Copy link
Contributor

@danvleju-rdx danvleju-rdx commented Sep 4, 2024

Jira ticket: ABW-3776

Description

This PR fixes the issue where users could not create a new persona by explicitly declaring the body. This was necessary due to a conflict between our FeatureReducer protocol and the @Reducer macro.

@matiasbzurovski
Copy link
Contributor

This looks good to me, but I insist we should do what I proposed on its PR: there is no point for this view to have a reducer with a ViewAction that its only logic is to inform the delegate via its only DelegateAction

Makes more sense to have it as a plain SwiftUI view that takes a closure instead.

Copy link
Contributor

@CyonAlexRDX CyonAlexRDX left a comment

Choose a reason for hiding this comment

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

oh scary! Have you gone through other (all? 😬 ) Reducers? We might have more bugs caused by this?!

@danvleju-rdx
Copy link
Contributor Author

@CyonAlexRDX Yes, all good. We have only 10 places (for now) where we use @Reducer macro.

@danvleju-rdx
Copy link
Contributor Author

@matiasbzurovski Agree with you. Should I do this change in this PR though?

@GhenadieVP
Copy link
Contributor

@danvleju-rdx @matiasbzurovski Let's do the improvement in a separate PR.

@GhenadieVP GhenadieVP merged commit cb79036 into main Sep 5, 2024
6 checks passed
@GhenadieVP GhenadieVP deleted the ABW-3776-unable-to-create-new-persona branch September 5, 2024 06:40
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants