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

Backport 'Fix edit form in intitiatives' to v0.26 #10782

Conversation

alecslupu
Copy link
Contributor

🎩 What? Why?

Backport #10724 to v0.26

♥️ Thank you!

@alecslupu alecslupu added backport Pull Requests that are a backport for a fixed bug module: initiatives type: fix PRs that implement a fix for a bug labels Apr 25, 2023
@alecslupu alecslupu added this to Pending review from Product in Maintainers via automation Apr 25, 2023
@alecslupu alecslupu moved this from Pending review from Product to Backport pending in Maintainers Apr 25, 2023
@ahukkanen
Copy link
Contributor

Can you please rebase this PR? There is a conflict because of #10786.

… backport/0.26/fix-edit-form-in-intitiatives-10724
ahukkanen
ahukkanen previously approved these changes Apr 26, 2023
@ahukkanen
Copy link
Contributor

@alecslupu The tests are failing here. It is trying to call .merge() for a string on this line:

public_send("#{name}=", field.merge(locale => super(value)))

The problem I believe is that the hash coercer in Virtus does not convert a string into a hash, you can try this in the console (using 0.26):

> Virtus::Attribute.build(Virtus::Attribute::Hash).coerce("Foobar")

In 0.27 it works somehow differently with the new attribute system as we don't have this exception happening.

@ahukkanen ahukkanen dismissed their stale review April 26, 2023 08:52

There are still some issue with the tests when using Virtus.

@ahukkanen
Copy link
Contributor

I investigated the problem a bit and I think there is a problem with the original fix that this is backporting.

This would be the proper way to handle the problem where the data is stored in multilingual format but the form only shows the inputs for the current locale:

self.title = translated_attribute(model.title)
self.body = translated_attribute(model.body)

And when saving the record (I believe this is already done in initiatives):

title: {
I18n.locale => title_with_hashtags
},

@alecslupu
Copy link
Contributor Author

I investigated the problem a bit and I think there is a problem with the original fix that this is backporting.

This would be the proper way to handle the problem where the data is stored in multilingual format but the form only shows the inputs for the current locale:

self.title = translated_attribute(model.title)
self.body = translated_attribute(model.body)

And when saving the record (I believe this is already done in initiatives):

title: {
I18n.locale => title_with_hashtags
},

@ahukkanen thanks for the input on fixing this. 35d372f fixes the issue

@ahukkanen
Copy link
Contributor

@ahukkanen thanks for the input on fixing this. 35d372f fixes the issue

Thanks @alecslupu but I think we need to do the same change at develop too before the backport. Using translated_attribute on the form is incorrect if the UI does not show the translated fields.

Why the current code works at develop is due to this backwards compatibility functionality:

# This preserves backwards compatibility for non-hash values e.g. with
# the admin hero content block which expects the images container object
# as the default value when building the admin form.
return value unless value.is_a?(::Hash)

In the core modules we should should avoid relying on backwards compatibility features.

@ahukkanen ahukkanen merged commit f5f1256 into release/0.26-stable May 4, 2023
Maintainers automation moved this from Backport pending to Done May 4, 2023
@ahukkanen ahukkanen deleted the backport/0.26/fix-edit-form-in-intitiatives-10724 branch May 4, 2023 06:43
@alecslupu alecslupu added this to the 0.26.7 milestone Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Pull Requests that are a backport for a fixed bug module: initiatives type: fix PRs that implement a fix for a bug
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants