feat: Context Groups with tree nesting and User Group ACL

[Ibochkarev]

What this does

Multilingual / multi-site installs can group Contexts in the manager tree and grant editors access to a whole group with one ACL (comment on #14422).

Tree / UI

• modContextGroup with rank; assign a Context via context_group (one group per Context for folder placement)
• CRUD on Contexts, optional tree folders + Context Group switcher

ACL

• modAccessContextGroup + User Group Permissions tab Context Groups
• Expands into the same modAccessContext policy bucket (by member Context key), so existing Context policies keep working
• Additive with per-Context ACL; gated by access_context_enabled
• First ACL on a group closes those Contexts to users without matching access (called out in the lexicon)

Membership safety (review feedback from @mkschell / @ShevArtV)

• Contexts is an aggregate, not a composite: deleting a group does not delete member Contexts
• Members are unassigned (context_group = 0) after a successful $group->remove()
• Group ACL rows still cascade with the group

How to test

1. Upgrade / run 3.3.0-context-groups so context_groups and access_context_groups exist.
2. Create a Context Group, assign two language Contexts.
3. User Groups → Permissions → Context Groups: add an ACL with a Context policy.
4. Log in as a non-admin editor in that User Group: both Contexts show; an ungrouped Context does not get access from the group ACL.
5. Confirm Administrator still reaches member Contexts after the first group ACL.
6. Delete a Context Group that still has members: Contexts stay in the DB and become ungrouped; group ACL rows go away.

Related

Closes #14422
Closes #16000

Also covers the ACL ask from #14422 (comment)

Compatibility

Upgrade creates context_groups, access_context_groups, and context.context_group. Existing per-Context ACL stays; group ACL is additive.

First Context Group ACL on a group can close previously open member Contexts for users without matching ACL (same class of behavior as the first per-Context ACL).

Tests

• _build/test/Tests/Model/Security/modAccessContextGroupTest.php
• _build/test/Tests/Processors/Security/Access/ContextGroupAclTest.php
• _build/test/Tests/Processors/Context/ContextGroupTest.php (includes direct $group->remove() keeping Contexts)

Design notes / follow-ups

Single FK (context.context_group) is intentional for the tree: one folder parent, simple switcher. That does not cover overlapping ACL groups (e.g. language + brand). Longer term I’d split tree placement from ACL membership and look at a Resource Group–style M:N for the ACL side. Open for discussion before another schema pass.

Also still out of scope here:

• Nesting Context Groups / ACL inheritance across groups
• Resource Group / Category / Media Source ACL via Context Groups
• ACL grid on the Context Group edit page
• Requiring access_permissions to change context_group on a Context

Contributors

@muzzwood (feature request)
@SnowCreative (ACL use case)

[mkschell]

It's not clear to me what parts of the original 5-7 year old feature discussion were implemented and which weren't, and how those decisions were made. There's no indication that alternate approaches were considered, and no invitation to discuss the approach on a technical and taste level before suddenly being faced with a 2900 line code review on top if it. It feels like the definition of "implementation" that Cursor did here includes synthesizing the entire approach. I'm a big user of AI myself, but I personally want to see more thoughtfulness from the human side. (Are we basically doing "find everything that's been discussed and requested in Revo and make it happen" here, or am I overreacting?)

That "Gate E" section in the PR makes no sense.

[Ibochkarev]

It's not clear to me what parts of the original 5-7 year old feature discussion were implemented and which weren't, and how those decisions were made. There's no indication that alternate approaches were considered, and no invitation to discuss the approach on a technical and taste level before suddenly being faced with a 2900 line code review on top if it. It feels like the definition of "implementation" that Cursor did here includes synthesizing the entire approach. I'm a big user of AI myself, but I personally want to see more thoughtfulness from the human side. (Are we basically doing "find everything that's been discussed and requested in Revo and make it happen" here, or am I overreacting?)

That "Gate E" section in the PR makes no sense.

Hi!

In this case, I took two related queries with milestones enabled. I reviewed the comments, created an implementation plan, and implemented them using AI. After each implementation phase, I tested and refined them (fixing any bugs) in the browser and using tests.

As with all my other PRs (most have milestones enabled).

I also added permissions in addition to the query itself.

While I have the opportunity and time, I try to devote a lot of time to reviewing the issue, analyzing it, and working on documentation (most of the MRs have already been merged by @Mark-H, and some are still under review).

That "Gate E" section in the PR makes no sense - Removed, thanks

[Ibochkarev]

Tell me how I can help you – break the deadlock in testing, releases, roadmaps, documentation, and so on!

I'll do everything I can. For now, I'm still optimistic and working on the PR.

But unfortunately, it's dwindling every day, as there haven't been any breakthroughs in six months.

Perhaps @opengeek is working on it, but unfortunately, the community doesn't know about it and is waiting for news and new versions with improvements and bug fixes that we've been living with for a very long time (some bugs are 10 years old).

I really want to make MODX better – but, as they say in Russia, one man is no match for another... but without support and changes in the core, teams won't make any progress.

No offense – I want the best! And if my work causes inconvenience and irritation for everyone, we can fork the Russian-speaking community... but I want to move forward together!

@Mark-H @theboxer @rthrash @theboxer @matdave fyi

[mkschell]

Thanks for your reply, @Ibochkarev I am certain that we all want to make MODX better, and doing it together seems like the best way. I should be careful to note, even though I work for MODX the company, my own comments and opinions on Revo issues, PRs and concerns about maintainer workload are my own and it's not my intention to represent an official stance of the company.

Back to the PR, I have some thoughts on a key design decision.

You model membership as a single foreign key — context.context_group — so a Context can belong to only one group. Did you consider a many-to-many design, like Resource Groups use? The concern: multi-site setups often need overlapping groups (for example, one Context in both a language group and a brand group). With a single key, those users must duplicate per-context ACLs again, which is the problem this feature is meant to solve.

A related schema question: Contexts is declared as a composite on modContextGroup, so a direct $group->remove() cascade-deletes the member Contexts. Your Remove processor works around this, but the model itself is not safe for other callers. Should this be an aggregate instead? A group does not really own its Contexts — membership is a loose association.

So my question is: was the single-key design a deliberate choice (maybe for the tree folders?), or mainly for simplicity? If the tree is the reason, my preference would be to decouple the tree view from membership and follow the Resource Group pattern for the ACL side. Happy to discuss before you invest in changes.

[Ibochkarev]

@mkschell Thanks — both points land.

The single FK was intentional for the tree (one folder per Context, easy switcher). ACL got bolted onto that, so overlapping groups (language + brand) aren’t covered. Long term I’d separate tree placement from ACL membership and lean toward a Resource Group–style M:N for the ACL side. Happy to talk through that before we redo the schema.

On the composite: agreed, a group doesn’t own Contexts. The Remove processor only papers over it. I’ll switch that relation to an aggregate on this PR unless you’d rather park it for the M:N redesign.

[ShevArtV]

I agree that a many-to-many relationship makes more sense here, given how user groups and resource groups work. Cascade deletion is also unnecessary — if a group is deleted, the contexts should just become ungrouped, not disappear. This PR is valuable and should be accepted.

[Ibochkarev]

@mkschell updated