Problem/Motivation

On admin/config/development/configuration/features the status is called 'Enabled' even this concept no longer exists in D8. We also have code-level usage of 'enabled', e.g., FeaturesManagerInterface::STATUS_ENABLED.

Proposed resolution

Update 'Enabled' to 'Installed' and FeaturesManagerInterface::STATUS_ENABLED to FeaturesManagerInterface::STATUS_INSTALLED.

Remaining tasks

To fix test failures, identify for occurrences of 'Enabled' that need to be changed to 'Installed' in FeaturesCreateUITest::testCreateFeaturesUI() and make the changes.

User interface changes

API changes

Data model changes

assignments.exclude.module.enabled configuration key renamed to assignments.exclude.module.installed.

CommentFileSizeAuthor
#23 interdiff-2631656-19-23.txt1.61 KBashhishhh
#23 2631656-23.patch5.93 KBashhishhh
#19 interdiff-2631656-11-19.txt1.58 KBashhishhh
#19 2631656-19.patch5.91 KBashhishhh
#11 2631656-11.patch4.25 KBashhishhh
#8 2631656-8.patch318 bytesashhishhh
#5 2631656-5.patch320 bytesfelribeiro
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner created an issue. See original summary.

nedjo’s picture

nedjo
he/him/his
Primary language English
CreditAttribution: nedjo at Chocolate Lily commented
Issue summary: View changes
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Issue tags: +Novice

.

felribeiro’s picture

felribeiro CreditAttribution: felribeiro at CI&T commented
Assigned: Unassigned » felribeiro
felribeiro’s picture

felribeiro CreditAttribution: felribeiro at CI&T commented
Status: Active » Needs review
FileSize
2631656-5.patch320 bytes

Status: Needs review » Needs work

The last submitted patch, 5: 2631656-5.patch, failed testing.

felribeiro’s picture

felribeiro CreditAttribution: felribeiro at CI&T commented
Status: Needs work » Needs review
ashhishhh’s picture

ashhishhh CreditAttribution: ashhishhh at Valuebound commented
FileSize
2631656-8.patch318 bytes

Your patch did not take care of white space.
I cleaned it.

ashhishhh’s picture

ashhishhh CreditAttribution: ashhishhh at Valuebound commented

.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented

IMHO ideally we would also rename the name of the constant.

ashhishhh’s picture

ashhishhh CreditAttribution: ashhishhh at Valuebound commented
FileSize
2631656-11.patch4.25 KB

Changed constant everywhere.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Status: Needs review » Reviewed & tested by the community

Cool, thank you! IMHO the next issue would be to rename state_disabled to state_uninstalled

The last submitted patch, 5: 2631656-5.patch, failed testing.

The last submitted patch, 8: 2631656-8.patch, failed testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 11: 2631656-11.patch, failed testing.

nedjo’s picture

nedjo
he/him/his
Primary language English
CreditAttribution: nedjo at Chocolate Lily commented
Status: Needs work » Reviewed & tested by the community
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented
Status: Reviewed & tested by the community » Needs work

Awesome, the test coverage we have strikes back!

nedjo’s picture

nedjo
he/him/his
Primary language English
CreditAttribution: nedjo at Chocolate Lily commented
Issue summary: View changes

@felribeiro, @ashhishhh: thanks for taking this issue. The test failures mean that the tests must be updated to use the new text that's introduced in this patch. I've updated the issue summary with remaining work.

ashhishhh’s picture

ashhishhh CreditAttribution: ashhishhh at Valuebound commented
FileSize
2631656-19.patch5.91 KB
interdiff-2631656-11-19.txt1.58 KB

Thanks @nedjo,
Worked on 

src/Tests/FeaturesCreateUITest.php

to correct tests as well.

ashhishhh’s picture

ashhishhh CreditAttribution: ashhishhh at Valuebound commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 19: 2631656-19.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner as a volunteer commented 
error: patch failed: src/FeaturesManager.php:684
error: src/FeaturesManager.php: patch does not apply
Patch Failed to apply

Some change in the meantime requires you to pull the new code first and then try to create another patch.

ashhishhh’s picture

ashhishhh CreditAttribution: ashhishhh at Valuebound commented
FileSize
2631656-23.patch5.93 KB
interdiff-2631656-19-23.txt1.61 KB

OK @dawehner

ashhishhh’s picture

ashhishhh CreditAttribution: ashhishhh at Valuebound commented
Status: Needs work » Needs review

  • nedjo committed 79cb2b5 on 8.x-3.x authored by ashhishhh 
    Issue #2631656 by ashhishhh, felribeiro, nedjo: Features should be...

  • nedjo committed 774a5c5 on 8.x-3.x 
    Issue #2631656 by nedjo: set assignments.exclude.module.installed...
nedjo’s picture

nedjo
he/him/his
Primary language English
CreditAttribution: nedjo at Chocolate Lily commented
Issue summary: View changes
Status: Needs review » Fixed

Thanks all. Added a few more changes and then committed.

Most significant change was the assignments.exclude.module.enabled configuration key renamed to assignments.exclude.module.installed, which breaks backward compatibility.

Status: Fixed » Closed (fixed)

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