This patch removes the use of radio buttons to select the flag, and in turn makes the flag a regular argument (UI-wise) as shown in snap.png. This allows more complex configurations which are discussed here #298109-73: Rules integration.

The patch also adds a 'Load flag' action. A follow-up patch can include 'Add a flag'.

CommentFileSizeAuthor
#4 flag_rules_flag_as_argument_4.patch9.27 KBamitaibu
#3 flag_rules_flag_as_argument_3.patch9.28 KBamitaibu
snap.png52.23 KBamitaibu
flag_rules_flag_as_argument.patch7.83 KBamitaibu

Comments

mooffie’s picture

mooffie commented

Isn't this a dup of #394152: Improvement for the Rules integration?

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
commented

oops, I forgot I opened that issue :) I've closed it now as duplicate.

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
commented
StatusFileSize
new flag_rules_flag_as_argument_3.patch9.28 KB

Some more cleanup, and fix of calling wrong function.

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
commented
Status: Needs review » Needs work
StatusFileSize
new flag_rules_flag_as_argument_4.patch9.27 KB

And this time the correct patch...

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
commented
Status: Needs work » Needs review

and correct status... sigh.

looplog’s picture

looplog commented
Component: Code » Flag core

Your suggestion at #545788: create new flag on node submit was unfortunately beyond my understanding of how to trigger php on node save, so rules integration in regards to your suggested 'Add a flag' action would be ideal for my uses. Was wondering if you've had any progress with this patch?

BenK’s picture

BenK commented

Subscribing....

Bilmar’s picture

Bilmar commented

+1 subscribing

quicksketch’s picture

quicksketch
he/him
commented

BenK and trupal218: Could you guys review this patch? I don't use or support Rules in any way, so the only way this will be committed will be if it is tested and reviewed by other users who use Rules. As a feature request, this sounds like something that will probably only go into the 2.x version, and so this patch will most likely need to be updated to match the latest code.

robby.smith’s picture

robby.smith commented

subscribing

robby.smith’s picture

robby.smith commented
Status: Needs review » Reviewed & tested by the community

Against 6.x-1.x-dev

patching file flag.module
Hunk #1 succeeded at 650 (offset 42 lines).
patching file includes/flag.rules.inc
Hunk #1 FAILED at 17.
1 out of 6 hunks FAILED -- saving rejects to file includes/flag.rules.inc.rej
patching file includes/flag.rules_forms.inc

Against 6.x-2.0-beta3

patching file flag.module
Hunk #1 succeeded at 1060 with fuzz 2 (offset 452 lines).
patching file includes/flag.rules.inc
Hunk #1 FAILED at 17.
Hunk #2 FAILED at 42.
Hunk #5 succeeded at 208 (offset -7 lines).
Hunk #6 succeeded at 228 (offset -7 lines).
2 out of 6 hunks FAILED -- saving rejects to file includes/flag.rules.inc.rej
patching file includes/flag.rules_forms.inc

Against 6.x-2.x-dev

patching file flag.module
Reversed (or previously applied) patch detected!  Assume -R? [n] 

Hunk #1 succeeded at 266 with fuzz 2 (offset -342 lines).
patching file includes/flag.rules.inc
Hunk #1 FAILED at 17.
Hunk #2 FAILED at 42.
Hunk #5 succeeded at 208 (offset -7 lines).
Hunk #6 succeeded at 228 (offset -7 lines).
2 out of 6 hunks FAILED -- saving rejects to file includes/flag.rules.inc.rej
patching file includes/flag.rules_forms.inc
Hunk #5 FAILED at 96.
1 out of 5 hunks FAILED -- saving rejects to file includes/flag.rules_forms.inc.rej
robby.smith’s picture

robby.smith commented
Status: Reviewed & tested by the community » Needs review

above is the output when using patch at #4 for me

mooffie’s picture

mooffie commented

Amitai, here's a hypothetical question (which is not related to the issue at hand): Would you accept ownership of, say, a "Flag Rules" module, if we decouple the Rules integration from the Flag module itself? I'm not saying we'll decouple it, but it'd be helpful to know if anybody would be interested in maintaining it.

As for the issue at hand:

- Backward compatibility: Will currect saved rules continue to work?
- Simplicity: Will we lose the ability to flag items by simply adding one action?

amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
commented

Moffie,

> Would you accept ownership of, say, a "Flag Rules"

I actually haven't used Rules for a long time so I'm less passionate about it, so sorry, but I'd have to refuse. Luckily it's only an hypothetical question ;)

joachim’s picture

joachim commented
Version: 6.x-1.x-dev » 7.x-3.x-dev
Component: Flag core » Rules integration
Assigned: amitaibu » Unassigned

> I don't use or support Rules in any way, so the only way this will be committed will be if it is tested and reviewed by other users who use Rules.

Heh. Ditto.

Status: Needs review » Needs work

The last submitted patch, flag_rules_flag_as_argument_4.patch, failed testing.

akalata’s picture

akalata commented
Status: Needs work » Postponed

Marking as postponed, given the need for a Rules-focused maintainer to work with Flag. See #1736524: Rules maintainer / decouple Rules integration to a new project

ivnish’s picture

ivnish
he/him
commented
Issue summary: View changes
Status: Postponed » Closed (outdated)

Closed as outdated because Drupal 7 is EOL