fix: expose RundownId of expectedPackage in publication to package-manager#1745
fix: expose RundownId of expectedPackage in publication to package-manager#1745Julusian wants to merge 3 commits into
Conversation
WalkthroughThe PR extends the expected packages publication system to include and propagate a ChangesExpected Packages rundownId Addition
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 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! |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
meteor/server/publications/packageManager/expectedPackages/generate.ts (1)
130-130: ⚡ Quick winAdd regression coverage for
rundownIdpropagation.Please add/extend a publication generation test that asserts
expectedPackage.rundownIdis forwarded intoexpectedPackageoutput (including the undefined/nullish case), since this path has regressed before.🤖 Prompt for 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. In `@meteor/server/publications/packageManager/expectedPackages/generate.ts` at line 130, Add a regression test that covers propagation of expectedPackage.rundownId through the publication generator: extend or create a unit/test in the publication generation tests that calls the same generator used in generate.ts (the function that builds expectedPackage outputs) and assert that the resulting expectedPackage object contains rundownId when input.expectedPackage.rundownId is set, and that the field is absent/undefined for nullish inputs; specifically target the code path that maps expectedPackage.rundownId -> expectedPackage in the generator to ensure both non-null and undefined/null cases are exercised.
🤖 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.
Nitpick comments:
In `@meteor/server/publications/packageManager/expectedPackages/generate.ts`:
- Line 130: Add a regression test that covers propagation of
expectedPackage.rundownId through the publication generator: extend or create a
unit/test in the publication generation tests that calls the same generator used
in generate.ts (the function that builds expectedPackage outputs) and assert
that the resulting expectedPackage object contains rundownId when
input.expectedPackage.rundownId is set, and that the field is absent/undefined
for nullish inputs; specifically target the code path that maps
expectedPackage.rundownId -> expectedPackage in the generator to ensure both
non-null and undefined/null cases are exercised.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 144ab2a3-b176-49c3-a39b-9f11598441df
📒 Files selected for processing (4)
.husky/pre-commitmeteor/server/publications/packageManager/expectedPackages/contentCache.tsmeteor/server/publications/packageManager/expectedPackages/generate.tspackages/shared-lib/src/package-manager/publications.ts
About the Contributor
This pull request is posted on behalf of the BBC
Type of Contribution
This is a: Bug fix
Current Behavior
During #1595, the rundownId property needed by package-manager to do rundown based prioritisation of packages was broken due to removing a property that appeared unused.
New Behavior
This restores the property
Testing
Affected areas
Time Frame
Other Information
Status