-
Notifications
You must be signed in to change notification settings - Fork 133
Add a cache tag to invalidate a group's user membership listings #483
Conversation
This also includes a block to display the member count of a group as a demonstration of the cache tag.
999f9ae
to
daf8ad5
Compare
src/Entity/OgMembership.php
Outdated
$tags = Cache::buildTags('og-group-membership-list', $this->getGroup()->getCacheTagsToInvalidate()); | ||
Cache::invalidateTags($tags); | ||
if ($group = $this->getGroup()) { | ||
$tags = Cache::buildTags('og-group-membership-list', $group->getCacheTagsToInvalidate()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe use a constant instead of 'og-group-membership-list'
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this is already merged. However, I still detected some points
* {@inheritdoc} | ||
*/ | ||
public function defaultConfiguration() { | ||
return [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We always have to account the parent return value, even in this case that returns an empty array. This is because the parent might return something in the future or at some point we switch to other base class:
return [
...
] + parent:: defaultConfiguration();
* Default template: og-member-count.html.twig. | ||
* | ||
* @param array $variables | ||
* An associative array containing: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The array contains also the membership_states
key but this is not detailed below.
* - count: An integer representing the number of members in the group. | ||
* - group: The group, which is a content entity. | ||
*/ | ||
function template_preprocess_og_member_count(array &$variables) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see the reason of this preprocess. We can pass the label directly from the block plugin.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is intended to demonstrate how and end user would change this block into something useful for them.
* Available variables: | ||
* - count: The number of members in the group. | ||
* - membership_states: An array of membership states included in the count. | ||
* - group_label: The group label. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The group
variable is also available as we don't unset it in template_preprocess_og_member_count()
. If we don't need this variable, we probably want to pass the label from the beginning, in the block plugin and remove the template preprocess function. Or we add here the group
too.
*/ | ||
protected function assertMemberCount($group_key, $expected_count) { | ||
$expected_string = (string) $this->formatPlural($expected_count, '@label has 1 member.', '@label has @count members', ['@label' => $this->groups[$group_key]->label()]); | ||
$this->assertTrue(strpos($this->renderBlock($group_key), $expected_string) !== FALSE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be written in more meaningful way:
$this->assertContains($expected_string, (string) $this->renderBlock($group_key));
@claudiu-cristea, please review this PR: #486 |
@MPParsley, thank you. I just did a review. I see only one aspect. |
Fixes #472.
This also includes a member count block that demonstrates the use of the cache tag, but is fully themeable and usable in production sites to display member counts in group pages.