Skip to content

feat(Scheduled Analysis): add scheduled analysis to the list of user-updatable parameters BED-7186#2299

Merged
sirisjo merged 3 commits intomainfrom
BED-7186
Jan 27, 2026
Merged

feat(Scheduled Analysis): add scheduled analysis to the list of user-updatable parameters BED-7186#2299
sirisjo merged 3 commits intomainfrom
BED-7186

Conversation

@sirisjo
Copy link
Contributor

@sirisjo sirisjo commented Jan 27, 2026

Description

Updates the list of parameters that are user-updatable to include the scheduled analysis parameter.

Motivation and Context

Resolves BED-7186

This is part of the Scheduled Analysis epic. We want scheduled analysis to be user-updatable via the API, instead of users having to go through TAMs or modify the DB directly to edit this param.

Also fixed some slog lint warnings in the parameters file.

How Has This Been Tested?

Tested locally, received 200 response.

Screenshots (optional):

Screenshot 2026-01-26 at 17 01 50

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

Summary by CodeRabbit

  • New Features

    • Added a new "Scheduled Analysis" configuration parameter that can now be managed.
  • Improvements

    • Converted many configuration warnings to structured logs with clearer fields for easier diagnosis and troubleshooting.
  • Bug Fixes

    • Corrected a log message typo ("password expiration") for clearer messaging.

✏️ Tip: You can customize this high-level summary in your review settings.

@sirisjo sirisjo self-assigned this Jan 27, 2026
@sirisjo sirisjo added the api A pull request containing changes affecting the API code. label Jan 27, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 27, 2026

Walkthrough

Adds a new public ParameterKey ScheduledAnalysis ("analysis.scheduled"), marks it valid and removes it from the protected set, and replaces many fmt-based warnings with structured slog warnings (including sanitized keys and error fields); also fixes a log typo.

Changes

Cohort / File(s) Summary
Parameter key & validation
cmd/api/src/model/appcfg/parameter.go
Added ScheduledAnalysis ("analysis.scheduled"). Updated IsValidKey to include it and removed it from IsProtectedKey so it is no longer protected.
Logging refactor (structured warnings)
cmd/api/src/model/appcfg/parameter.go
Replaced fmt-based warnings with structured slog warnings across parameter retrieval/validation functions; added structured fields (e.g., invalid_configuration, parameter_key), sanitized parameter keys in logs, and fixed a typo ("password expiratio" → "password expiration").

Sequence Diagram(s)

(omitted — changes are localized and do not introduce a multi-component sequential flow)

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested reviewers

  • Useinovski
  • urangel

Poem

🐰 A little key hopped into place,
Logs grew tidy, with clearer grace.
Scheduled checks now join the fray,
Warnings whisper the proper way.
A rabbit twitches — code's in place! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main change: adding scheduled analysis to user-updatable parameters, with a reference to the ticket BED-7186.
Description check ✅ Passed The description covers key sections including Description, Motivation/Context, How Has This Been Tested, and Types of Changes, with a completed checklist.

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

✨ Finishing touches
  • 📝 Generate docstrings

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.

slog.WarnContext(ctx, "Failed to fetch CitrixRDPSupport configuration; returning default values")
} else if err := cfg.Map(&result); err != nil {
slog.WarnContext(ctx, fmt.Sprintf("Invalid CitrixRDPSupport configuration supplied, %v. returning default values.", err))
slog.WarnContext(ctx, "Invalid CitrixRDPSupport configuration supplied, returning default values.", slog.String("invalid_citrix_rdp_support_configuration", err.Error()))
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for improving the logs here! I actually think we can go a step further if you don't mind, the slog string keys are helpful to hinge filters on, what do you think about making all of these the same something like invalid_configuration and the value remains the error, and then adding another slog.String("parameter_key", ::value_is_the_key::) that way one can query logs for all or specific config loading issues?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can do!

Copy link
Contributor

@mistahj67 mistahj67 left a comment

Choose a reason for hiding this comment

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

Code LGTM, tested locally as well 🚀

@sirisjo sirisjo merged commit 14a7ec1 into main Jan 27, 2026
13 checks passed
@sirisjo sirisjo deleted the BED-7186 branch January 27, 2026 18:43
@github-actions github-actions bot locked and limited conversation to collaborators Jan 27, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api A pull request containing changes affecting the API code.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants