Problem/Motivation
SubscribeMenuLink::getTitle() returns "Upgrade" when the current user has a subscription and "Join" otherwise, but getCacheContexts() declares only user.roles. Subscription ownership is per-user, not per-role-combination: two authenticated users with identical roles (common before any subscription-granted role, i.e. exactly the population deciding whether to join) share a render-cached menu entry, so whichever renders first fixes the label for the other. A subscriber can see "Join", or a non-subscriber "Upgrade".
Steps to reproduce
- Two users with the same roles, one subscribed (whose plan grants no role, or before role grant), on a site with render-cached menus.
- Load pages as each; both see the label produced for whoever primed the cache.
Proposed resolution
Declare the user cache context, and add the subscription_list cache tag via getCacheTags() so the label updates when subscriptions change (the module already invalidates that tag on subscription insert/update/delete). While in the file, wrap the two titles in $this->t() and have SubscriptionManagerController::subscribeTitle() reuse the same strings, fixing their missing translatability in one place.
Remaining tasks
Patch; extend SubscribeMenuLinkTitleTest to assert contexts/tags.
User interface changes
Correct per-user Join/Upgrade labels; strings become translatable.
API changes
New SubscriptionManagerService::subscribeLabel(AccountInterface): TranslatableMarkup. SubscribeMenuLink's constructor takes the current user and the subscription manager (autowired). core_version_requirement raised from ^10 || ^11 to ^11.3.
Data model changes
None.
Issue fork subscription_manager-3616936
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
colanComment #4
colanFixed in the MR, in two commits, with one scope addition and one policy change worth flagging.
The menu link.
SubscribeMenuLinknow declares theusercache context (notuser.roles) and thesubscription_listcache tag, so a user's own label also flips when they subscribe. Thecache_contextskey inlinks.menu.ymlis removed:MenuLinkBasenever read it, so it only documented an intention. The label strings move to a newSubscriptionManagerService::subscribeLabel(AccountInterface $account): TranslatableMarkup, which bothgetTitle()and the subscribe page's_title_callbackuse, so they cannot disagree.Same bug, one page over. The subscribe page itself had the same class of problem:
subscribe()redirects subscribers to their connector but renders the connector chooser for everyone else, and the chooser's render array declared no cache metadata while the route has nono_cache. Dynamic Page Cache would therefore have served one user's chooser page (title "Join" included) to a subscriber with the same permissions who should have been redirected. The chooser build now declares theusercontext and thesubscription_list,subscription_plan_listandconfig:subscription_manager.settingstags — who sees it depends on their own subscription, what it lists on the visible plans and the chooser settings.Autowiring, and Drupal 10.
SubscribeMenuLinkwas pulling its dependencies through\Drupal::calls. Since core 11.3PluginBase::create()autowires plugin constructors (andMenuLinkDefaultno longer overrides it), so the plugin now just declares a constructor with the current user and the subscription manager (the latter via#[Autowire(service: 'subscription_manager')], as the service has no interface alias). On Drupal 10 that constructor would fatal, becauseMenuLinkDefault::create()there still passes exactly four arguments. Rather than keep a hand-written factory for a core version that reaches end of life in about two months, alongside Drupal 12's release,core_version_requirementbecomes^11.3. Drupal 10 was nominal support anyway — nothing in this roadmap ever ran the module on it, and the CI tests only the current major. The#[LegacyHook]shims that existed for 10 can go in the cleanup omnibus (#3616937: Cleanup omnibus: dead config keys, missing token dependency, per-role user saves, page_attachments overhead, drush alias, field length, translatability, misc), along with autowiring the remaining plugins. Drupal 12 support is a separate roadmap step; 1.0.0 is realistically a Drupal 12-era release.Tests.
SubscribeMenuLinkTitleTestnow builds the plugin throughcreate()as the menu system does, and gains two cases: two users with identical roles get different labels from one link instance, which declaresuser(and notuser.roles) andsubscription_list; and the labels areTranslatableMarkup, with the page title callback agreeing with the menu link in both states.ConnectorChooserTestasserts the chooser build's contexts and tags. Suite at 164 kernel tests.Change record drafted (the core requirement change is the user-visible part).
Comment #10
colan