Problem/Motivation

There is no administrative way to initiate a flag reset. This must be done programmatically.

Proposed resolution

Create a new entity operation that adds reset to the operations popup in the flag list page.

Remaining tasks

Create, test patch.

User interface changes

An additional reset operation would be available in the admin UI

API changes

None.

CommentFileSizeAuthor
#14 interdiff-14.txt1.13 KBjoshi.rohit100
#14 flagResetAdminUI-2491489-14.patch4.27 KBjoshi.rohit100
#13 flagResetAdminUI-2491489-13.patch4.24 KBmartin107
#13 interdiff-4-13.txt1.04 KBmartin107
#4 interdiff-3-4.txt375 bytesmartin107
#4 flagResetAdminUI-2491489-4.patch3.2 KBmartin107
#3 flagResetAdminUI-2491489-3.patch3.2 KBsocketwench
#2 flagResetAdminUI-2491489-2.patch2.71 KBsocketwench

Comments

socketwench’s picture

socketwench commented

Not sure if this is a regression or not since it's been ages since I've used Flag 7.x...

socketwench’s picture

socketwench commented
StatusFileSize
new flagResetAdminUI-2491489-2.patch2.71 KB

Pretty sure this is still broken.

Definitely still broken. When you select "Reset" from the operations popup, you get an attempt to call a function from a non-object in EntityConfirmFormBase lin 26. For some reason, the flag entity isn't being passed to the entity confirm form...

socketwench’s picture

socketwench commented
StatusFileSize
new flagResetAdminUI-2491489-3.patch3.2 KB

Hm. Guess I can't use EntityConfirmFormBase there...

martin107’s picture

martin107 commented
StatusFileSize
new flagResetAdminUI-2491489-4.patch3.2 KB
new interdiff-3-4.txt375 bytes

I really like this issue ...

With my users hats on adding to the operation pull down button is exactly where I would first go to look for it ... so that is great

there is a minor error in the case of the name FlagResetForm::getFormID() - it need to match up FormInterface::getFormId()

When I manually test I am getting errors from inside flag_reset_flag() when trying to access the no longer existent $flag->flag_id

My instinct is to wait for #2478565: flag_reset_flag() is broken which should fix that.

socketwench’s picture

socketwench commented

My instinct is to wait for #2478565: flag_reset_flag() is broken which should fix that.

Normally I'd agree, but I needed to have a non-programmatic way to invoke the reset so I could debug #2478565: flag_reset_flag() is broken.

joachim’s picture

joachim commented

> Not sure if this is a regression or not since it's been ages since I've used Flag 7.x...

On D7 and prior, reset was only something available to Rules and through the API. There's never been a UI for it (though it's been requested, there's an old issue somewhere).

socketwench’s picture

socketwench commented

Seems to make sense to me. Might as well include it now since it's mostly written anyways.

socketwench’s picture

socketwench commented
Status: Active » Needs review

The last submitted patch, 2: flagResetAdminUI-2491489-2.patch, failed testing.

socketwench queued 4: flagResetAdminUI-2491489-4.patch for re-testing.

socketwench’s picture

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

Tested #4 locally in concert with #2488782: Define a FLAG_RESET event and kill hook_flag_reset().. Everything looks good.

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
commented
Status: Reviewed & tested by the community » Needs work 
  1. +++ b/src/Form/FlagResetForm.php
@@ -0,0 +1,87 @@
+    return t('Are you sure you want to reset the Flag %label?', [
+      '%label' => $this->flag->label(),
+    ]);

    Should be $this->t() 

  2. +++ b/src/Form/FlagResetForm.php
@@ -0,0 +1,87 @@
+    return t('All flaggings created with Flag %label will be deleted.', [
+      '%label' => $this->flag->label(),

    again $this->t() 

  3. +++ b/src/Form/FlagResetForm.php
@@ -0,0 +1,87 @@
+    return t('Reset');
...
+    drupal_set_message(t('Flag %label was reset.', [
+      '%label' => $this->flag->label(),

    $this->t() 

  4. +++ b/src/Form/FlagResetForm.php
@@ -0,0 +1,87 @@
+    drupal_set_message(t('Flag %label was reset.', [
+      '%label' => $this->flag->label(),

    $this->t()

martin107’s picture

martin107 commented
Status: Needs work » Needs review
StatusFileSize
new interdiff-4-13.txt1.04 KB
new flagResetAdminUI-2491489-13.patch4.24 KB

Fixed.

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
commented
StatusFileSize
new flagResetAdminUI-2491489-14.patch4.27 KB
new interdiff-14.txt1.13 KB

OOps FlagResetForm missed the $this->t() changes.

martin107’s picture

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

Thanks for catching my mistake.

  • socketwench authored e217d1e on 8.x-4.x 
    Issue #2491489 by martin107, socketwench, joshi.rohit100, joachim: Added...
socketwench’s picture

socketwench commented
Status: Reviewed & tested by the community » Fixed

Thanks everyone!

socketwench’s picture

socketwench commented

@martin107: Technically, fixing the t() in FlagDeleteForm was scope creep and probably should have been in another patch. It's a pretty small change this time, so *shrug*.

Status: Fixed » Closed (fixed)

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