department-of-veterans-affairs / va.gov-cms

Editor-centered management for Veteran-centered content.
https://prod.cms.va.gov
GNU General Public License v2.0
97 stars 69 forks source link

UX review of System-wide alerts fieldset on VAMC System Operating Status #14820

Open xiongjaneg opened 1 year ago

xiongjaneg commented 1 year ago

User Story or Problem Statement

As a VAMC editor, I need to be able to easily update my VAMC System Operating Status and not be prevented from saving my changes due to unpublished VAMC System Banner Alerts.

As a VAMC editor, I still want to see VAMC System Banner Alerts listed on the VAMC System Operating Status node for convenience (so just removing this whole fieldset is not a viable option).

Description or Additional Context

To Reproduce

Steps to reproduce the behavior:

Acceptance Criteria

davidmpickett commented 1 year ago

error_message

xiongjaneg commented 1 year ago

@swirtSJW Could you provide some background for UX review (and my understanding) on the purpose of this fieldset?

swirtSJW commented 1 year ago

The inline entity browser let you see all the banners assigned to a system and allowed editors to edit them or remove them. It is problematic for many reasons.

  1. An editor has access to edit a banner that may have been created for multiple systems and just happens to include their system.
  2. They can remove themselves from a group banner and the last one to remove, leaves a published banner behind that shows no-where because it is assigned to no systems.
  3. Now that we have allow only one, preventing duplicate combinations of system assignment, we end up with a case where a group banner assigned to A,B,F can no longer be saved when F is removed becuase there is already a banner for A,B. Or same is true with no assignments. There can be only one assigned to nothing... although there never should be but it is caused by the "remove" button.

We likely should list them, but make them click the link to see the banner and then can decide if they need to edit. THen they will get proper allow_only_one errors that tell them others exist and link to them. If we have an edit button, it should take them too the node edit rather than let them edit it right there. Easiest lift is no edit button and just link to the existing banners.

davidmpickett commented 1 year ago

Discussed in UX refinement 8/24

swirtSJW commented 1 year ago

Path forward is remove this fieldset, update relevant KB articles and monitor issues from help desk.

I think that is killing off too much functionality. If we take it away completely, they have no way to know it is there, and no way to opt out of it, if it no longer applies. We could have the list just not have the remove button, only a link to the banner. That would let them see it in context and not have the other drawbacks.

davidmpickett commented 1 year ago

If we take it away completely, they have no way to know it is there

Just created an example multi-system banner on staging to test a theory.

screencapture-staging-cms-va-gov-visn-15-vamc-banner-alert-2023-08-24-example-secret-banner-2023-08-24-22_10_21

Logging in as a local editor, I can use the VAMC alerts and status view to find the multi-system banner published on my system.

screencapture-staging-cms-va-gov-admin-content-vamc-alerts-and-statuses-2023-08-24-22_11_12

xiongjaneg commented 1 year ago

We could have the list just not have the remove button, only a link to the banner. That would let them see it in context and not have the other drawbacks.

@davidmpickett Any concerns about this option?

davidmpickett commented 1 year ago

We could have the list just not have the remove button, only a link to the banner. That would let them see it in context and not have the other drawbacks.

@davidmpickett Any concerns about this option?

Multiple. Maybe a topic for 16th minute?

xiongjaneg commented 1 year ago

Proposal from Steve: Don't include Archived banners Remove Remove button Remove Edit button Only include a link to the actual node where editing should happen

xiongjaneg commented 1 year ago

From Dave: This is not a pattern that is common across Drupal. KB articles don't refer to the banner information here.

mmiddaugh commented 1 year ago

Trying to wrap my head around the various use cases here and posting a few questions in advance of the meeting later this week.

Steve, your comment indicates removing the inline editor will prevent the editor from knowing about the alert or opting out of a banner which no longer applies and suggests keeping the list but removing the Remove button. Wouldn't removing the remove button also prevent the editor from opting out of the banner?


Now that we have allow only one, preventing duplicate combinations of system assignment, we end up with a case where a group banner assigned to A,B,F can no longer be saved when F is removed because there is already a banner for A,B.

Question: What's the use case in which a banner created and applied to 3 facilities might later only apply to 2 of the facilities?


We could have the list just not have the remove button, only a link to the banner. That would let them see it in context and not have the other drawbacks.

Question: If an editor uses the link to see the banner in context, what actions are available from that view? Could the it be edited or removed? (And is either action desirable?)

davidmpickett commented 1 year ago

Steve, your comment indicates removing the inline editor will prevent the editor from knowing about the alert or opting out of a banner which no longer applies and suggests keeping the list but removing the Remove button. Wouldn't removing the remove button also prevent the editor from opting out of the banner?

It is unclear if the remove button currently does anything other than removing an item from this list. In my testing on staging/testing the VAMC Banner Alerts that I "removed" were still active and unaffected. But I also am not sure if there would need to be a content release for those actions to be reflected.

davidmpickett commented 1 year ago

Question: If an editor uses the link to see the banner in context, what actions are available from that view? Could the it be edited or removed? (And is either action desirable?)

It would take them to the node view page. The actions available will depend on the Sections of the Editor and the Banner. In this screenshot, I am pretending to be a VAMC editor viewing a banner that has been added at a higher level and all I can do is see who created it. I have no actions because the banner is in a section I am not

screencapture-staging-cms-va-gov-visn-15-vamc-banner-alert-2023-08-24-example-secret-banner-2023-08-24-22_10_21

davidmpickett commented 1 year ago

Question: What's the use case in which a banner created and applied to 3 facilities might later only apply to 2 of the facilities?

Want to emphasize that our main reason for overhauling banners was to reduce/prevent instances of multiple banners.

Multisystem banners are already a rare case. I just did a quick visual scan of all 800 VAMC Banners that have ever existed and only found 2 instances of multisystem banners - both of which were related to PACT Act.

xiongjaneg commented 1 year ago

Decision per Michelle, remove the system-wide alerts fieldset from VAMC System Operating Status. @davidmpickett will include in this ticket brief instructions for alternate methods of viewing banner alerts information. @xiongjaneg will stub out a ticket for KB article updates on alternate methods of viewing.

swirtSJW commented 1 year ago

Removing field_banner_alert is going to have FE implecations since that is the field that FE uses to decide what to show on the templates image

swirtSJW commented 1 year ago

Removing it is considerably more work. This is not a field set. This is an entity reference to the content that is supposed to be on the operating status page. The build method from the FE is going to need to change.

swirtSJW commented 1 year ago

Sorry, the horror of what is actually happening with this field is now revealing itself.

The checkboxes on the System banner alert are the source of truth for where the banners appear. The presence of the system banner in this field (field_banner_alert) controls what shows on the Operating status page, not the banner itself.

The two were acting as separate sources of truth.

davidmpickett commented 1 year ago

Removing the fieldset from view by VAMC editors =/= removing the fields from the content model

swirtSJW commented 1 year ago

It is not that simple.... because it is not as automated as we thought.

The two are not connected in the way we thought they were.... which is bad. We can not simply hide the field and have the problem go away.

I need to contemplate this a bit more to reduce the impact to the other systems.

swirtSJW commented 1 year ago

This is the bit I was missing. It is only syncing the changes in one direction, not both. That is why clicking the remove button did not actually clear the banner check-box image

image

swirtSJW commented 1 year ago

Oyy this goes back a long ways https://github.com/department-of-veterans-affairs/va.gov-cms/pull/634 (41 commits and 144 files changed :( )There is no issue to refer to because it predates our use of github for issues. We were using Jira at the time.

swirtSJW commented 1 year ago

After sleeping on this for a bit. I think we should remove the field and have the FE switch to using the reverse entity reference from VAMC System Banner Alert with Situation Updates->field_banner_alert_vamcs rather than the entity reference from VAMC System Operating Status->field_banner_alert

xiongjaneg commented 1 year ago

Removing non-UX ACs from this tickets to create new tickets for those disciplines

swirtSJW commented 1 year ago

Removing the ief "remove" button removes the ability for a system editor to remove a broader, multi-system banner from their system.

This was an intentional feature (not sure how many used it) for a local editor to be able to opt out of a multi system banner. They would now have to email the multi-system editor to ask them to remove the banner from their system.

xiongjaneg commented 1 year ago

@swirtSJW I didn't bring your comments over to #15047 but referred to this ticket instead. If helpful, I can copy over your comments to that one.