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

[#53704] Visible=false project attribute values are deleted when a non-admins edit the attributes #15296

Conversation

dombesz
Copy link
Contributor

@dombesz dombesz commented Apr 17, 2024

https://community.openproject.org/work_packages/53704

The problem

When a custom field is hidden (visible=false) for non-admins (usually for project admins), it is cleared out while the project custom fields are updated from the sidebar. This happens because the hidden field is not submitted with the form, and the acts_as_customizable plugin will clear all the fields that are not submitted.

The solution

In order to fix the issue, we should also include the hidden fields in the backend when assigning the updated custom fields. To make this happen, we have to return the hidden fields too from the available_custom_fields method, when the _query_available_custom_fields_on_global_level is set to true. This flag signals to the plugin, that we want to request the available_custom_fields globally as opposed to requesting the project enabled custom fields.

Due to the complexity of the acts_as_customizable plugin and the plugin patch, the fix causes 1 test to fail. The test ensures, we do not activate hidden fields when they are being submitted by non-admin users. This contradicts the bug we are fixing here, because we have to assign the hidden field on the backend in order to not lose its value and its activated state.
This is not a critical issue, because it does not manifest on the frontend, and it will not allow setting any value for the hidden field either. I will provide a fix for the failing testcase in the next minor release.

@dombesz dombesz force-pushed the bug/53704-visible=false-project-attribute-values-are-deleted-when-a-non-admin-user-edits-the-attributes branch 2 times, most recently from cbd02e5 to 69012c5 Compare April 17, 2024 11:42
@dombesz dombesz added this to the 14.0.x milestone Apr 19, 2024
@dombesz dombesz force-pushed the bug/53704-visible=false-project-attribute-values-are-deleted-when-a-non-admin-user-edits-the-attributes branch from 69012c5 to de85bc6 Compare April 19, 2024 07:50
@dombesz dombesz marked this pull request as ready for review April 19, 2024 09:45
dombesz added 7 commits April 19, 2024 11:45
…ok and also disable non available custom fields.

We have a few callbacks that interfered with the
disable_custom_fields_with_empty_values being on the after_create
callback. For example the CustomValue model also has an after_create
callback activate_custom_field_in_customized_project which overwrites
the changes we do in the disable_custom_fields_with_empty_values. This
makes all the custom fields with a custom value to be activated,
regardless of the changes we do in the
disable_custom_fields_with_empty_values. Since we want the
disable_custom_fields_with_empty_values to have the final word, it is
moved into a before_commit hook, which will be executed after the
activate_custom_field_in_customized_project.
…ommit hook and also disable non available custom fields."

This reverts commit de85bc6.
This is a compromise solution to avoid further patching the acts as
customizable plugin, because it will be removed anyway. This behaviour
does not manifest on the UI, so it does not present a real problem.
@dombesz dombesz force-pushed the bug/53704-visible=false-project-attribute-values-are-deleted-when-a-non-admin-user-edits-the-attributes branch from 499a0ff to db7942c Compare April 19, 2024 09:45
@ulferts ulferts self-requested a review April 19, 2024 12:10
Copy link
Contributor

@ulferts ulferts left a comment

Choose a reason for hiding this comment

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

There is a tiny typo but apart from this, the change looks good, @dombesz. Merge at will.

app/models/projects/acts_as_customizable_patches.rb Outdated Show resolved Hide resolved
@dombesz dombesz merged commit 80974a0 into release/14.0 Apr 19, 2024
9 checks passed
@dombesz dombesz deleted the bug/53704-visible=false-project-attribute-values-are-deleted-when-a-non-admin-user-edits-the-attributes branch April 19, 2024 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants