Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.At the moment, we have lots of methods on the Flag entity that are in pairs, like:
- getFlagLongText()
- getUnflagLongText()
This doesn't seem like good DX to me, because code using these always has to something like this:
$title = $action === 'unflag' ? $flag->getUnflagShortText() : $flag->getFlagShortText();
This is unwieldy.
It would be nicer to be able to do:
$title = $flag->getShortText($action);
and let the internals return the right one of the flag/unflag variants.
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | interdiff.2868098.10-12.txt | 5.86 KB | joachim |
| #12 | 2868098-12.flag_.refactor-all-getunFlag-methods.patch | 31.12 KB | joachim |
| #10 | interdiff-2868098-8-10.txt | 8.52 KB | martin107 |
| #10 | getShorty-2868098-10.patch | 28.65 KB | martin107 |
Comments
Comment #2
martin107 commentedI think this is good idea.
Comment #4
martin107 commentedI will fix this up soon.
Comment #5
martin107 commentedComment #6
martin107 commentedIn the next iteration ... I have fixed up testing and the forms.
Comment #8
martin107 commentedButter fingers!
Comment #10
martin107 commentedOh rats I missed conversion of
getUnflagShortText()
Comment #11
martin107 commentedComment #12
joachim commentedThanks for working on this. I didn't realize there were so many calls -- hope you used find & replace! :)
Here's a new patch:
- couple of whitespace nitpicks
- changed the form to use the plain get() method, which I think makes it easier to read as it's the same property name as the form element. The form is tied to the innards of the FlagType class anyway in the names of the form elements, so hiding internals here isn't really relevant.
- also handled the flag message methods. (And in doing discovered a WTF: #2870279: [meta] [regression] Flag message is not used anywhere)
Comment #13
martin107 commentedThat is a good idea... it was something that was making me feel grumpy about my own solution.
In terms of review I have had a slow visual scan of the patch .. everything looks in order / better.
so +1 from me.
Comment #14
joachim commentedGood enough for me -- we have too many patches needing review and they're holding up further work, so I'm going to say let's go with this.