The changes introduced in #2809081: Default profile handling is inconsistent introduce a regression, reverting some good logic introduced in #2799669: Only show "Save" instead of both buttons when first profile.. New profile forms now contain a "Safe and set default" button even when "allow multiple" is unchecked, which makes no sense from a user perspective. New profile forms seem to have isActive() set to true.

Not sure where to fix this - by setting isActive() to false on any new form, or reverting to some of the logic that was previously in place to hide that button when isNew() is true.

Comments

dafeder created an issue. See original summary.

mglaman’s picture

First step: Let's write a test to show default button is there
Second step: Hide "Set default" if isNew

dafeder’s picture

Issue summary: View changes
dafeder’s picture

OK, new to test writing for D8 but will try to create a patch.

mglaman’s picture

dafeder thanks for trying! There's plenty of existing tests, can plug a new assertion into an existing test. You can find my in IRC/Drupal slack and ping for help during the week if you want.

mglaman’s picture

Assigned: Unassigned » mglaman

Working on test to prove the bug.

mglaman’s picture

Version: 8.x-1.0-alpha7 » 8.x-1.x-dev
Status: Active » Needs review
StatusFileSize
new1.85 KB

This test should fail.

Status: Needs review » Needs work

The last submitted patch, 7: set_default_button_now-2878235-7.patch, failed testing.

mglaman’s picture

Status: Needs work » Needs review
StatusFileSize
new2.88 KB

Here is passing patch. This checks if the profile type supports multiple. And we have logic which ensures inactive profiles are never default.

  • mglaman committed 749bba1 on 8.x-1.x
    Issue #2878235 by mglaman, dafeder: Set default button now showing in...
mglaman’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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