feat: migrate meteor settings file to new places#1781
Open
Julusian wants to merge 5 commits into
Open
Conversation
Julusian
added
the
Contribution from SuperFly.tv
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
WalkthroughRemoves the Meteor settings-file-based ChangesSettings File Removal and DB Settings Migration
Sequence Diagram(s)sequenceDiagram
rect rgba(100, 149, 237, 0.5)
Note over Env,koa.ts: Server startup
participant Env as SOFIE_ENABLE_HEADER_AUTH
participant auth as auth.ts (ENABLE_HEADER_AUTH)
participant koa as koa.ts
participant MeteorConfig as __meteor_runtime_config__
end
rect rgba(144, 238, 144, 0.5)
Note over ClientSettings,UserPermissions: Client init
participant ClientSettings as client/lib/Settings.ts
participant UserPermissions as UserPermissions.tsx
end
rect rgba(255, 165, 0, 0.5)
Note over CoreSystemDB,SystemMgmtUI: Settings persistence
participant CoreSystemDB as CoreSystem DB
participant getCoreSystemSettings as getCoreSystemSettings()
participant SystemMgmtUI as SystemManagement.tsx
end
Env->>auth: parsed into ENABLE_HEADER_AUTH boolean
auth->>koa: ENABLE_HEADER_AUTH
koa->>MeteorConfig: inject enableHeaderAuth into __meteor_runtime_config__
MeteorConfig->>ClientSettings: APP_HEADER_AUTH_ENABLED = MeteorInjectedSettings.enableHeaderAuth
ClientSettings->>UserPermissions: gates permission fetch effects
SystemMgmtUI->>CoreSystemDB: writes confirmKeyCode / poisonKey / maximumDataAge
CoreSystemDB->>getCoreSystemSettings: applyAndValidateOverrides(settingsWithOverrides)
getCoreSystemSettings->>UserPermissions: (separate flow)
getCoreSystemSettings->>TriggersHandler: reactive poisonKey
getCoreSystemSettings->>ModalDialog: confirmKeyCode at bind time
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@meteor/server/api/cleanup.ts`:
- Around line 86-89: The maximumDataAge variable extracted from systemSettings
on line 88 is not validated before being used in cleanup threshold calculations.
Add validation logic to ensure maximumDataAge is a positive finite number before
it is applied to any retention or cleanup math. If the value is invalid (less
than or equal to zero, NaN, or Infinity), either clamp it to a safe minimum
value or fallback to the DEFAULT_MAXIMUM_DATA_AGE constant to prevent unsafe
aggressive purging of recent data across collections.
In `@packages/documentation/docs/user-guide/configuration/sofie-core-settings.md`:
- Line 7: In the opening sentence of the sofie-core-settings.md file, replace
the contraction "it's" with the possessive form "its" in the phrase "configured
at its most basic level". The sentence should read "_Sofie Core_ is
configured at its most basic level using environment variables." where "its"
indicates possession rather than "it is".
In `@packages/shared-lib/src/core/model/StudioSettings.ts`:
- Around line 131-133: The new field `defaultShelfDisplayOptions` added to
`IStudioSettings` in StudioSettings.ts needs to be mapped in the REST conversion
layer to prevent it from being silently dropped in API responses. Add the
mapping for `defaultShelfDisplayOptions` in both the `APIStudioSettingsFrom`
function (which converts to API format) and the `APIStudioSettingsTo` function
(which converts from API format) in the type conversion module to ensure proper
round-trip serialization and deserialization of this setting.
In `@packages/webui/src/client/lib/viewPort.ts`:
- Around line 225-227: The variable followOnAirSegmentsHistory is being used
directly as a loop counter without normalization, but since it can contain
fractional values (like 1.5), this causes the loop to iterate an incorrect
number of times. Before assigning followOnAirSegmentsHistory to the loop counter
variable i, normalize it to a non-negative integer by using Math.floor to remove
any fractional part and ensure it doesn't go below zero.
In `@packages/webui/src/client/ui/SegmentTimeline/TimelineGrid.tsx`:
- Line 46: The new prop defaultDisplayDuration added at line 46 is not being
monitored for changes in the component's update guards. Update the
shouldComponentUpdate method to include defaultDisplayDuration in its prop
comparison logic, and ensure that componentDidUpdate also invalidates the
relevant cache (total duration cache) when defaultDisplayDuration changes. This
will prevent the grid from using stale duration values when this prop is
updated.
In `@packages/webui/src/client/ui/Settings/Studio/Generic.tsx`:
- Around line 511-526: The defaultTimeScale property uses IntInputControl which
truncates decimal values via parseInt, but the property is modeled as a number
that should support decimal precision. Replace IntInputControl with a
FloatInputControl or equivalent numeric control that properly handles and
preserves decimal values when updating the defaultTimeScale key through the
handleUpdate callback.
In `@packages/webui/src/client/ui/Shelf/Shelf.tsx`:
- Around line 600-604: The displayOptions variable assignment in the Shelf
component does not guard against params['display'] being a string array (which
occurs when URL has repeated display parameters). The .split() method will fail
at runtime if params['display'] is an array. Add a type check to handle both
cases: if params['display'] is already an array, use it directly; if it's a
string, call .split(',') on it. Ensure the result is always an array of strings
before assigning to displayOptions, falling back to
studioSettings?.defaultShelfDisplayOptions or DEFAULT_SHELF_DISPLAY_OPTIONS as
needed.
🪄 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: d32d0b38-9289-4099-9125-982e6ec713f1
⛔ Files ignored due to path filters (1)
packages/openapi/src/generated/openapi.yamlis excluded by!**/generated/**
📒 Files selected for processing (47)
meteor/server/Settings.tsmeteor/server/__tests__/cronjobs.test.tsmeteor/server/api/cleanup.tsmeteor/server/api/rest/koa.tsmeteor/server/api/rest/v1/typeConversion.tsmeteor/server/api/studio/api.tsmeteor/server/api/systemTime/ntpTimeChecker.tsmeteor/server/coreSystem/index.tsmeteor/server/lib/rest/v1/studios.tsmeteor/server/migration/1_40_0.tsmeteor/server/migration/upgrades/system.tsmeteor/server/security/auth.tspackages/documentation/docs/user-guide/configuration/sofie-core-settings.mdpackages/documentation/docs/user-guide/features/access-levels.mdpackages/meteor-lib/src/Settings.tspackages/openapi/api/definitions/studios.yamlpackages/shared-lib/src/core/constants.tspackages/shared-lib/src/core/model/CoreSystemSettings.tspackages/shared-lib/src/core/model/StudioSettings.tspackages/webui/src/client/collections/index.tspackages/webui/src/client/lib/ModalDialog.tsxpackages/webui/src/client/lib/Settings.tspackages/webui/src/client/lib/rundown.tspackages/webui/src/client/lib/rundownTiming.tspackages/webui/src/client/lib/triggers/TriggersHandler.tsxpackages/webui/src/client/lib/ui/containers/modals/Modal.tsxpackages/webui/src/client/lib/viewPort.tspackages/webui/src/client/ui/App.tsxpackages/webui/src/client/ui/ClockView/ClockView.tsxpackages/webui/src/client/ui/Prompter/PrompterView.tsxpackages/webui/src/client/ui/RundownView.tsxpackages/webui/src/client/ui/RundownView/MediaStatusPopUp/MediaStatusPopUpItem.tsxpackages/webui/src/client/ui/RundownView/MediaStatusPopUp/index.tsxpackages/webui/src/client/ui/RundownView/RundownDetachedShelf.tsxpackages/webui/src/client/ui/RundownView/RundownHeader/useRundownPlaylistOperations.tsxpackages/webui/src/client/ui/RundownView/RundownTiming/RundownTimingProvider.tsxpackages/webui/src/client/ui/RundownView/RundownViewContextProviders.tsxpackages/webui/src/client/ui/SegmentStoryboard/SegmentStoryboard.tsxpackages/webui/src/client/ui/SegmentTimeline/Constants.tsxpackages/webui/src/client/ui/SegmentTimeline/Renderers/VTSourceRenderer.tsxpackages/webui/src/client/ui/SegmentTimeline/SegmentTimeline.tsxpackages/webui/src/client/ui/SegmentTimeline/SegmentTimelineContainer.tsxpackages/webui/src/client/ui/SegmentTimeline/TimelineGrid.tsxpackages/webui/src/client/ui/Settings/Studio/Generic.tsxpackages/webui/src/client/ui/Settings/SystemManagement.tsxpackages/webui/src/client/ui/Shelf/Shelf.tsxpackages/webui/src/client/ui/UserPermissions.tsx
💤 Files with no reviewable changes (3)
- meteor/server/Settings.ts
- meteor/server/api/systemTime/ntpTimeChecker.ts
- packages/webui/src/client/ui/App.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
About the Contributor
This pull request is posted on behalf of Superfly
Type of Contribution
This is a: Code improvement / Documentation improvement
Current Behavior
There are some sofie settings defined in a
meteor-settings.jsonconfig file. This is documented but is not very friendly to those configuring sofie, as it requires to be done at deployment separate from the rest of the settings.New Behavior
These settings have all been migrated to new homes:
The
customizationClassNamesetting has been removed, as I am not sure if it has value. I expect that anyone wanting to use this already needs to embed custom css into sofie, in which case they can easily do this in another way. Please say if you disagree and use this.The
enableNTPTimeCheckersetting has been removed as it appears it was broken and has been for I suspect multiple years.Testing
Affected areas
Time Frame
Other Information
Status