Problem/Motivation

Following #3538360: Profile & Instance naming normalization

In Profile entity & ProfileForm, we have enable property.

This may be wrong for 2 reasons:

  1. it must be "enabled" instead of "enable" because it is a status and not an action
  2. In Drupal Core, there is only one plugin with this mechanism and it is using status boolean property instead:
$ grep  -A 1 -r '$enabled' web/core/lib/Drupal/Core/*/Attribute
$ grep -A 1 -r '$enabled' web/core/modules/*/src/Attribute
$ grep  -A 1 -r '$status' web/core/lib/Drupal/Core/*/Attribute
$ grep -A 1 -r '$status' web/core/modules/*/src/Attribute
web/core/modules/filter/src/Attribute/Filter.php:   * @param bool $status
web/core/modules/filter/src/Attribute/Filter.php-   *   (optional) Whether this filter is enabled or disabled by default.

Proposed resolution

Renaming may not be enough. We also need to check if we can leverage existing mechanisms related to the standard status property (used in many config entities: Action, Date, Menu, Views, Search page, View mode, Language...) and simplify our code.

Issue fork display_builder-3545219

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

pdureau created an issue. See original summary.

mogtofu33’s picture

mogtofu33 commented
Issue tags: +beta blocker
smovs’s picture

smovs commented
Assigned: Unassigned » smovs
pdureau’s picture

pdureau commented
Issue summary: View changes

smovs opened merge request !123

pdureau’s picture

pdureau commented

I am not sure backward compatibility with "status" is expected because we are still in alpha phase and not commited yet to storage stability.

smovs’s picture

smovs commented
Assigned: smovs » Unassigned
Status: Active » Needs review

Hi team!
I renamed "Enable" to "Enabled".
Also, I added code to allow compatibility with the old name. This is temporary and needs to be removed later.

Please review MR

pdureau’s picture

pdureau commented
Issue summary: View changes
Status: Needs review » Needs work

HI @smovs,

Thanks for you MR. 2 feedbacks:

  • I don't think backward compatibility with "status" is expected because we are still in alpha phase and not commited yet to storage stability.
  • I realize my issue description may be confusing, because I said "enabled" would be better than "enable" but I was proposing "status" instead because no plugin in Core uses "enabled".

Those 2 feedbacks are open to discussion if needed ;)

smovs’s picture

smovs commented
Status: Needs work » Needs review

Refactored MR according to the suggestions. Please review

pdureau’s picture

pdureau commented
Assigned: Unassigned » mogtofu33
Status: Needs review » Reviewed & tested by the community

Looks good to me

mogtofu33’s picture

mogtofu33 commented
Status: Reviewed & tested by the community » Needs work

Playwright test failing seems to show a PHP error, need to investigate.
Rebase is a bit tricky, could you do it or create an other squashed branch?

mogtofu33’s picture

mogtofu33 commented
Assigned: mogtofu33 » Unassigned
smovs’s picture

smovs commented
Assigned: Unassigned » smovs
smovs’s picture

smovs commented
Assigned: smovs » Unassigned
Status: Needs work » Needs review

Indeed, I missed an issue in the ComponentLibraryPanel when I renamed 'status' to 'component_status'.
Thanks @mogtofu33, for noticing this issue.

I fixed it and also fixed merge conflicts with 1.0.x branch.

Please review

pdureau’s picture

pdureau commented
Assigned: Unassigned » mogtofu33

  • mogtofu33 committed befaa2f5 on 1.0.x authored by smovs 
    [#3545219] feat: Profile entity: From enable to status

By: pdureau
By:...
mogtofu33’s picture

mogtofu33 commented
Assigned: mogtofu33 » Unassigned
Status: Needs review » Fixed

Now that this issue is closed, please review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, please credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.