Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
It is possible to substitute a module's service with an alternative representation. PHPUnit apparently does this a lot to create mock services. This should be possible for FlagService, but that class currently does not have an interface.
Proposed resolution
Create an interface for FlagService.
Remaining tasks
Create patch.
User interface changes
None.
API changes
No changes, only the addition of the interface.
Comment | File | Size | Author |
---|---|---|---|
#7 | interface-2475579-6.patch | 12.58 KB | martin107 |
#4 | interdiff-3-4.txt | 823 bytes | martin107 |
#4 | interface-2475579-4.patch | 12.23 KB | martin107 |
#3 | interdiff-3.txt | 1.01 KB | joshi.rohit100 |
#3 | interface-2475579-3.patch | 12.12 KB | joshi.rohit100 |
Comments
Comment #1
martin107 CreditAttribution: martin107 commentedI am fan of unit testing, this lays the foundations.
Comment #2
martin107 CreditAttribution: martin107 commentedSimple cut and paste exercise.
I wanted to leave both FlagService and FlagServiceInterface in good shape - so I have taken some liberties :-
1) If I were to create an initial template using a drush generate:service command then I notice it adds an @package token so I have done that here.
2) I have reordered the use statements into alphabetical order.
Comment #3
joshi.rohit100With some changes also I dont know what should be done with testcases. I also found that in ActionLinkTypeBase, we have declared FlagService is unused but we have declared it.
Comment #4
martin107 CreditAttribution: martin107 commentedThanks for spotting that.
What follows is just minor documentation tidy-ups.
Comment #5
joachim CreditAttribution: joachim commented@package is very rarely used in core, and doesn't feature at all on https://www.drupal.org/coding-standards/docs.
Also, unfortunately this has clashed with #2471435: params of FlagService::getFlagging() should match getFlaggings().
Comment #7
martin107 CreditAttribution: martin107 commented1) reroll
2) removed @package
Comment #8
joachim CreditAttribution: joachim commented> I also found that in ActionLinkTypeBase, we have declared FlagService is unused but we have declared it.
This doesn't seem to have been taken care of -- which is perfectly fine, as it doesn't really belong in this issue. I'll file a side-issue: #2477091: ActionLinkTypeBase imports classes that it doesn't use.
Committed. Thanks everyone!