Problem: if I modify a rule there is no way I can undo/cancel my changes.

Solution: we should implement a cancel button like views_ui does which just deletes the modified rule from the temporary storage. Then it is in its original state again.

Comments

klausi created an issue. See original summary.

dasjo’s picture

dasjo
he/him
Primary language German
Location Zurich
commented
Issue tags: +Novice
kiranjoshi’s picture

kiranjoshi commented
Assigned: Unassigned » kiranjoshi
kiranjoshi’s picture

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

Submitted pull request

https://github.com/fago/rules/pull/363

Sentei’s picture

Sentei commented
Assigned: Unassigned » Sentei
Sentei’s picture

Sentei commented
Status: Needs review » Reviewed & tested by the community

The cancel button works, the temporary changes are deleted when cancel is being clicked.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Reviewed & tested by the community » Needs work

Can you add a Functional test to TempStorageTest to check that the cancel button works? I think a new test method there should demonstrate that the cancel button works.

markie’s picture

markie commented
Status: Needs work » Needs review

Added PR https://github.com/fago/rules/pull/370 to insert test as requested. Had to add result text to test.

dasjo’s picture

dasjo
he/him
Primary language German
Location Zurich
commented
Assigned: Sentei » Unassigned
kiranjoshi’s picture

kiranjoshi commented

Added Tests for:
* Clicking cancel displays the text "Canceled"
* Clicking cancel returns to the main Rules screen
* Clicking cancel does not cancel temporary changes if another user is editing and has lock the rule

I'm not sure if more tests should be added to verify the temp storage is actually removed.

PR https://github.com/fago/rules/pull/371

fago’s picture

fago
Primary language German
Location Vienna
commented
Status: Needs review » Fixed

Thanks, I think that's fine as test coverage for now! Committed.

While testing I ran into #2659806: Warnings when adding an action, but that does not seem to be related to this change.

  • fago committed 6ea2f72 on 8.x-3.x authored by KiranJoshi 
    Issue #2654816 by KiranJoshi, Sentei, markie: Add cancel button to undo...
klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

The test case added here was a bit wrong, I fixed it in #2659954: TempStorageTest::testLocking has random fails.

Status: Fixed » Closed (fixed)

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