Page MenuHomePhabricator

Display "impactful change" dialog and disable implementations on input / output change of existing function
Closed, ResolvedPublic

Description

As a logged in User modifying the input or output of an existing function, I would like a dialogue to be displayed so that I can truly understand the impact of my actions.

To do:

  • Show a dialog when an user try to change a input / output of an existing function
  • Ensure that the dialog does not show for NEW function creation
  • On "confirm change" click, ensure the implementation and testers are detached from the current function
  • On success removal of the implementation from the "local" object, show a toast to inform the user that the implementation and testers have been disabled

Note: It is ok to place the Toast whenever you can, we will create another ticket to be able to place the toast in the required position

Event Timeline

DVrandecic renamed this task from Display "inpactful change" dialog and disable implementations on input / output change of existing function to Display "impactful change" dialog and disable implementations on input / output change of existing function.Feb 22 2022, 5:18 PM

Change 821217 had a related patch set uploaded (by AdesojiThisDot; author: AdesojiThisDot):

[mediawiki/extensions/WikiLambda@master] Display "impactful change" dialog and disable implementations on input / output change of existing function

https://gerrit.wikimedia.org/r/821217

The first three points look good, however I'm unable to see the toast described in the fourth point. Is this implemented? if so, where should I be looking?

The last point is merged with the first, because of the way the removing happens we don't want to remove and potentially add back on the frontend if the user clicks 'No' on the toast. so what I did was update the initial toast messaging to state that both the implementation and testers will be removed if the user saves the edit.

I am fine with discussing this further if you feel we need to do it that way.

Change 821217 merged by jenkins-bot:

[mediawiki/extensions/WikiLambda@master] Display "impactful change" dialog and disable implementations on input / output change of existing function

https://gerrit.wikimedia.org/r/821217

gengh reopened this task as Open.

@gengh @AdesojiThisDot I have a couple of questions in order to review this patch:

  1. How destructive is this action? Once an editor changes the input and/or output type/s of a function, is there no going back?
  2. Once an editor changes an input and/or output type/s of a function, are the implementations and/or tests detached, disabled, removed or deleted? I'm asking because:
    • This task description reads "implementation and testers are detached from the current function", and then "inform the user that the implementation and testers have been disabled". Detach and disable means two different things.
    • While on beta cluster the dialog message reads "implementations and testers to be removed". Remove means something different from detach and disable.
  3. Would also be possible to customize the message based on the editor's edits? As an example: "You changed the {input | output | input and output} {type | types} of this function. This change will permanently delete all function implementations and tests. Do you want to proceed? Please be certain."

Just as a reference here's the dialog for deleting a repository on GitHub.

Screen Shot 2022-09-08 at 13.27.08.png (752×942 px, 140 KB)

@AAlhazwani-WMF For the first point, once you save/publish the function, there's no going back except you edit it again to the previous values.
I think the right word would be detached, and yes we can customize the message. I will create a separate ticket to handle this.

there's no going back except you edit it again to the previous values.

If you re-edit back to the previous input/output values, will the system automatically re-attach the function implementations/tests or would the editor have to do the attachment manually? Thanks @AdesojiThisDot!

No, another edit will behave exactly like the first one: all implementations and testers will be (or kept) detached. The editor with the right privileges will have to attach them manually.

Thanks @gengh! With this info I would suggest to slightly change the copy as the following:

Are you sure?

You changed the {input | output | input and output} {type | types} of this function. All existing implementations and tests will be detached from the current function. Do you want to proceed?

Continue editing

Proceed and publish

After reviewing the design I would also suggest to create a task to update the dialog as soon as T284838: [EPIC] Add Dialog component to Codex lands in prod. Here's how it will look:

Screen Shot 2022-09-09 at 10.53.07.png (1×3 px, 339 KB)

Specs on Figma too https://www.figma.com/file/bao6AHRGlXrzcl3TWNLUZy/Function-editor%3A-Impactful-changes-dialog?node-id=0%3A1

Thank you @AAlhazwani-WMF!

I proceed to close this task :)