Skip to content

NDM DRM-106: documentation for co-managed global setting - #1725

Merged
wtokarz-nwx merged 4 commits into
devfrom
ndm/DRM-106-co-managed-global-setting-documentation
Oct 20, 2026
Merged

wtokarz-nwx merged 4 commits into
devfrom
ndm/DRM-106-co-managed-global-setting-documentation

Conversation

@wtokarz-nwx

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions

Copy link
Copy Markdown
Contributor

Auto-Fix Summary

25 issues fixed, 3 skipped across 2 files

Category Fixes
OxfordComma (rewrite) 1
WhetherOrNot (rewrite) 1
Dale: ing-analysis 1
Dale: misplaced-modifiers 1
Dale: passive-voice 17
Dale: wordiness 4
Skipped (needs manual review) Reason

| docs/directorymanager/11.1/admincenter/applications/portal/server/advanced.md:36 — Dale: passive-voice | 'searches ... are based on the global catalog' is stative; an active rewrite risks changing meaning |
| docs/directorymanager/11.1/admincenter/applications/portal/server/advanced.md:71 — Dale: passive-voice | 'if the department isn't specified' — the actor (admin or sync source) is ambiguous |
| docs/directorymanager/11.1/portal/generalfeatures/portal.md:142 — Dale: wordiness | Repeated 'You can, however, set this page to display those ... too' pattern across sections is legacy text; rewording all instances is a broader editorial change |

Ask @claude on this PR if you'd like an explanation of any fix.

@jth-nw
jth-nw deployed to development October 19, 2026 14:28 — with GitHub Actions Active

@MarcinBukowskiAtNetwrix MarcinBukowskiAtNetwrix left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It says co-managed groups appear if either the global setting or the user's own option is│ No changes this session
on. The code doesn't work that way (UserSettingsHelper.GetUserOptionOrSetting, UserSettingsHelper.cs:23). A user's │
saved value always wins, and the global setting only applies when the user has never saved the option. A user who │
turned the option off will not see co-managed groups even when the global setting is on

@wtokarz-nwx

Copy link
Copy Markdown
Contributor Author

It says co-managed groups appear if either the global setting or the user's own option is│ No changes this session on. The code doesn't work that way (UserSettingsHelper.GetUserOptionOrSetting, UserSettingsHelper.cs:23). A user's │ saved value always wins, and the global setting only applies when the user has never saved the option. A user who │ turned the option off will not see co-managed groups even when the global setting is on

It seems like the user checkbox saves under a different key (Search.DisplayExchangeCoManagedGroups, HomeController.cs:956) from the one the global setting reads (Search.DisplayCoManagedGroupsInMyGroups), so no saved user value can override the global one, and GroupManagerService.cs:5315 then ORs the global flag with the user's setting, meaning co-managed groups show if either is on.

@github-actions

Copy link
Copy Markdown
Contributor

Documentation PR Review

Editorial Review

docs/directorymanager/11.1/admincenter/applications/portal/server/advanced.md

  • Clarity — Line 51: The new row says co-managed groups (Exchange additional owners) don't appear by default. The Display Nested Ownership row two lines above (line 49) says that when that toggle is disabled, My Groups already shows groups where the user is the "primary owner, additional owner, or Exchange additional owner." Readers get two different answers about whether Exchange additional owners see their groups by default. Suggested fix: Confirm the actual behavior and make the two rows agree. For example, drop "or Exchange additional owner" from the Display Nested Ownership row, or explain how the two settings relate.
  • Clarity — Line 51: This row says the setting applies to My Groups, My Deleted Groups, My Expired Groups, and My Expiring Groups. The matching user option in portal.md (line 225) mentions only the My Groups page. The setting name ("...in My Groups") also suggests a single tab. Suggested fix: Confirm which tabs the setting affects and use the same scope on both pages. If it covers all four tabs, say so in portal.md too.
  • Completeness — Line 51: The row says the setting depends on the messaging provider supporting Exchange additional owners. It doesn't say the option is limited to Active Directory identity stores, which the new note in portal.md (line 237) states. Suggested fix: Add "This setting applies only to Active Directory identity stores, and it takes effect only if the identity store's messaging provider supports Exchange additional owners."
  • Structure — Line 51: The row explains the admin setting/user option interaction ("co-managed groups appear if either one is enabled. Disabling this setting doesn't hide..."), and the note at line 60 repeats it. The row is already long for a table cell. Suggested fix: Keep the interaction in one place. Either cut the note's second sentence, or move the interaction out of the table row and into the note.
  • Clarity — Line 70: The edited sentence keeps "his or her account." Suggested fix: "asks the user to enroll their account."

docs/directorymanager/11.1/portal/generalfeatures/portal.md

  • Clarity — Line 226: The same role has a different name on each page. This section says "Exchange co-managed owner" and "co-managed owner," but advanced.md says "Exchange additional owner." Readers may think these are different roles. Suggested fix: Pick one term and use it on both pages, for example: "You can also set this page to display groups for which you are an Exchange additional owner (co-managed owner)."
  • Clarity — Line 225–226: "set this page to display those groups too for which you are..." is hard to read. Suggested fix: "You can also set this page to display groups for which you are an Exchange additional owner."
  • Structure — Line 237: The note opens with a prerequisite: the option is available only for Active Directory identity stores, and only when the messaging provider supports Exchange additional owners. It appears after the steps, so readers on other identity stores follow the steps before learning the option doesn't apply to them. Suggested fix: Move the first paragraph of the note above Step 1, as a plain sentence or its own :::note. Keep the admin-setting paragraph after the steps.
  • Completeness — Line 223: The new section adds a user option, but the Reset Defaults table at the end of the page doesn't list it. Readers can't tell what the option resets to. Suggested fix: Add a row to the table: "Display exchange co-managed groups | Not selected."

Summary

9 editorial suggestions across 2 files. Vale and Dale issues are auto-fixed separately.


What to do next:

Comment @claude on this PR followed by your instructions to get help:

  • @claude fix all issues — fix all editorial issues
  • @claude help improve the flow of this document — get writing assistance
  • @claude explain the voice issues — understand why something was flagged

You can ask Claude anything about the review or about Netwrix writing standards.

Automated fixes are only available for branches in this repository, not forks.

@github-actions

Copy link
Copy Markdown
Contributor

Auto-Fix Summary

16 issues fixed, 4 skipped across 2 files

Category Fixes
Dale: passive-voice 8
Dale: wordiness 8
Skipped (needs manual review) Reason

| docs/directorymanager/11.1/admincenter/applications/portal/server/advanced.md:49 — Dale: passive-voice | 'When the toggle button is disabled' / 'When this setting is enabled' describe a setting's state; rewording risks changing UI meaning |
| docs/directorymanager/11.1/admincenter/applications/portal/server/advanced.md:51 — Dale: passive-voice | 'When the setting is enabled/disabled' describe a setting's state rather than an action; left as is |
| docs/directorymanager/11.1/admincenter/applications/portal/server/advanced.md:71 — Dale: passive-voice | 'if the department isn't specified' has no clear actor (admin, sync, or user); rewrite would be speculative |
| docs/directorymanager/11.1/portal/generalfeatures/portal.md:9 — Dale: idioms | 'fine-tune' is a common technical term; replacing it is a judgment call |

Ask @claude on this PR if you'd like an explanation of any fix.

@wtokarz-nwx
wtokarz-nwx merged commit 910dde5 into dev Oct 20, 2026
10 checks passed

This branch was successfully deployed

1 active deployment
development — f1d7fdd9 Deployed Oct 20, 2026 by jth-nw via build-and-deploy #4640
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants