Skip to content

add missing data retention settings#8917

Merged
esethna merged 3 commits intomasterfrom
missing-data-ret-settings
Apr 28, 2026
Merged

add missing data retention settings#8917
esethna merged 3 commits intomasterfrom
missing-data-ret-settings

Conversation

@esethna
Copy link
Copy Markdown
Contributor

@esethna esethna commented Apr 24, 2026

OPened a new PR to clean up stale edits from: #8169

add missing data retention settings
@esethna esethna marked this pull request as ready for review April 24, 2026 15:37
Copilot AI review requested due to automatic review settings April 24, 2026 15:37
@esethna esethna requested a review from Combs7th April 24, 2026 15:39
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 24, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 37c9f359-b05f-4a90-b592-2d24f737218d

📥 Commits

Reviewing files that changed from the base of the PR and between bd1d5fd and cd300c2.

📒 Files selected for processing (1)
  • source/administration-guide/configure/compliance-configuration-settings.rst
🚧 Files skipped from review as they are similar to previous changes (1)
  • source/administration-guide/configure/compliance-configuration-settings.rst

📝 Walkthrough

Walkthrough

Adds documentation entries for Data Retention execution-rate and batch-control settings and three boolean deletion toggles, including their config.json paths, environment variables, defaults, non-negative integer constraints, and behaviour notes that toggles default to false and prevent deletion when false.

Changes

Cohort / File(s) Summary
Documentation: Compliance Data Retention
source/administration-guide/configure/compliance-configuration-settings.rst
Added docs for TimeBetweenBatchesMilliseconds, RetentionIdsBatchSize, BatchSize, BoardsRetentionDays (paths, env vars, defaults, non-negative integer constraints) and boolean toggles EnableMessageDeletion, EnableFileDeletion, EnableBoardsDeletion with default=false and behaviour notes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title directly matches the main change: adding missing data retention settings documentation.
Description check ✅ Passed The description references cleaning up stale edits from a prior PR and updating documentation for data retention settings, which aligns with the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch missing-data-ret-settings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@source/administration-guide/configure/compliance-configuration-settings.rst`:
- Line 155: The text "positive integer (≥ 0)" is self-contradictory; determine
whether zero is allowed for this setting and replace the phrase consistently in
the document: if 0 is allowed change to "non-negative integer (≥ 0)", otherwise
change to "positive integer (> 0)"; update every occurrence of that phrase (the
lines containing "Default is 100 milliseconds. Possible values are any positive
integer (≥ 0).", and the other two occurrences noted) so the wording and the
parenthetical inequality agree.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a8ab182a-1328-4125-83cb-48141dda31a7

📥 Commits

Reviewing files that changed from the base of the PR and between c4ab710 and bd1d5fd.

📒 Files selected for processing (1)
  • source/administration-guide/configure/compliance-configuration-settings.rst

Comment thread source/administration-guide/configure/compliance-configuration-settings.rst Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds missing DataRetentionSettings documentation entries to the Mattermost compliance configuration reference, cleaning up leftover work from #8169 and ensuring admins can discover/tune data retention job behavior from the docs.

Changes:

  • Documented additional DataRetentionSettings options in the Compliance > Data Retention Policies section.
  • Added config setting blocks for batch timing, batch sizing, boards retention days, and deletion enablement toggles.

Comment thread source/administration-guide/configure/compliance-configuration-settings.rst Outdated
Comment thread source/administration-guide/configure/compliance-configuration-settings.rst Outdated
Comment thread source/administration-guide/configure/compliance-configuration-settings.rst Outdated
Comment thread source/administration-guide/configure/compliance-configuration-settings.rst Outdated
@github-actions
Copy link
Copy Markdown
Contributor

Newest code from mattermost has been published to preview environment for Git SHA bd1d5fd

@mattermost mattermost deleted a comment from Copilot AI Apr 24, 2026
@esethna
Copy link
Copy Markdown
Contributor Author

esethna commented Apr 24, 2026

@claude please address the review comments above

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 24, 2026

Claude finished @esethna's task in 49s —— View job


Addressing Review Comments

  • Read the changed file to understand current content
  • Fix contradictory "positive integer (≥ 0)" wording at line 155
  • Fix contradictory "positive integer (≥ 0)" wording at line 173
  • Fix contradictory "positive integer (≥ 0)" wording at line 281
  • Add cross-reference to EnableBoardsDeletion near line 189
  • Commit and push changes

All review comments have been addressed in commit cd300c2:

  1. Lines 155, 173, 281 — Changed "positive integer (≥ 0)" to "non-negative integer (≥ 0)" to resolve the self-contradictory wording (a positive integer is > 0, but these settings allow 0).
  2. Line 189 — Added a cross-reference to EnableBoardsDeletion in the boards retention days description, so readers know boards deletion must also be enabled for the day-count setting to take effect.

…ference

- Change "positive integer (≥ 0)" to "non-negative integer (≥ 0)" in three places
- Add cross-reference to EnableBoardsDeletion in boards retention days description

Co-authored-by: Eric Sethna <esethna@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown
Contributor

Newest code from mattermost has been published to preview environment for Git SHA cd300c2

@esethna esethna merged commit 1270d5f into master Apr 28, 2026
5 checks passed
@esethna esethna deleted the missing-data-ret-settings branch April 28, 2026 22:26
@github-actions
Copy link
Copy Markdown
Contributor

Newest code from mattermost has been published to preview environment for Git SHA b0fee7f

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.

3 participants