Skip to content

add missing channel and edge case tests to mergeConfigurations#3240

Open
riya-kaurav wants to merge 2 commits intoRocketChat:masterfrom
riya-kaurav:test/merge-configurations-edge-cases
Open

add missing channel and edge case tests to mergeConfigurations#3240
riya-kaurav wants to merge 2 commits intoRocketChat:masterfrom
riya-kaurav:test/merge-configurations-edge-cases

Conversation

@riya-kaurav
Copy link

@riya-kaurav riya-kaurav commented Mar 8, 2026

What

Added 4 missing test cases to mergeConfigurations in main.spec.ts

Why

The following scenarios were previously untested:

  • App-level channel configuration merging
  • User channel override when configurable
  • Forced app config blocking user channel override
  • Existing skippedUpdateVersion preserved when no new skip provided

Testing

All 211 existing + new tests pass with yarn test

Summary by CodeRabbit

  • Accessibility
    • Enhanced screen reader compatibility in Download Manager with descriptive labels across all action buttons, improving navigation and clarity for accessibility-focused users
    • Improved Update dialog accessibility with descriptive labels on Skip, Remind Later, and Install buttons for better screen reader support

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 8, 2026

Walkthrough

These changes add accessibility labels to UI components and expand test coverage for update configuration merging logic, without altering any control flow or existing functionality.

Changes

Cohort / File(s) Summary
UI Accessibility Improvements
src/ui/components/DownloadsManagerView/DownloadItem.tsx, src/ui/components/UpdateDialog/index.tsx
Added ARIA labels to ActionButton and Button components across different states and actions for improved screen reader support.
Test Coverage Expansion
src/updates/main.spec.ts
Added 134 lines of new test cases validating channel merging behavior, including channel precedence, forced configuration handling, and null/empty value handling.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

community, type: chore

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'add missing channel and edge case tests to mergeConfigurations' directly and accurately summarizes the main change: adding new test cases for the mergeConfigurations function to cover channel-related behavior and edge cases.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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
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: 4

🧹 Nitpick comments (1)
src/updates/main.spec.ts (1)

144-145: Drop the leftover insertion note.

This comment is now just noise inside the suite. The tests are already in the describe block, so the code is clearer without it.

Suggested cleanup
-// Add these inside describe('mergeConfigurations') block
-

As per coding guidelines, "Write self-documenting code with clear naming; avoid unnecessary comments except for complex business logic or non-obvious decisions".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/updates/main.spec.ts` around lines 144 - 145, Remove the leftover
insertion note comment inside the describe('mergeConfigurations') test suite in
src/updates/main.spec.ts; locate the stray comment text (the "// Add these
inside describe('mergeConfigurations') block" marker) and delete it so the test
file contains only the actual tests and pre-existing comments, keeping the
describe('mergeConfigurations') block self-contained and uncluttered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/ui/components/DownloadsManagerView/DownloadItem.tsx`:
- Around line 185-229: The JSX block using ActionButton (handlers: handleRemove,
handleCopyLink, handlePause, handleCancel, handleResume, handleShowInFolder,
handleRetry) is failing prettier; run Prettier (or your project's format script)
on this component (DownloadItem.tsx) to reformat the ActionButton elements and
their aria-label props consistently so the file passes prettier/prettier and
ESLint/TypeScript checks, then re-commit the formatted file.

In `@src/ui/components/UpdateDialog/index.tsx`:
- Around line 92-103: The Button prop formatting is inconsistent (extra
whitespace) and breaks Prettier; fix the three Button JSX elements so props are
consistently ordered and spaced and the install Button's attributes are on one
line or following project JSX wrapping rules; specifically adjust the Button
elements where onClick handlers call handleSkipButtonClick,
handleRemindLaterButtonClick, and the install Button that uses
ref={installButtonRef} and onClick={handleInstallButtonClick} to remove the
stray space before aria-label and ensure proper indentation/wrapping so
ESLint/Prettier/TypeScript all pass.

In `@src/updates/main.spec.ts`:
- Around line 146-270: The newly added test block in src/updates/main.spec.ts is
misformatted and violates Prettier/ESLint rules; run the project formatter
(prettier) or your editor's format command on the file and fix
indentation/spacing for the inserted tests (those new it(...) blocks that use
mergeConfigurations) so the file passes lint/TS checks; ensure consistent
indentation, trailing commas and alignment with existing tests, then re-run lint
and unit tests before pushing.
- Around line 243-258: The test titled "handles null skip value in user
configuration" is actually exercising the omitted-skip case because
userConfiguration is an empty object; update the test to match the scenario by
either renaming the test to reflect "skip omitted" or modify the setup so
userConfiguration explicitly sets skippedUpdateVersion: null to cover the
null-path; adjust the assertion expectations accordingly and reference the test
name string and the variables defaultConfiguration, appConfiguration, and
userConfiguration when making the change.

---

Nitpick comments:
In `@src/updates/main.spec.ts`:
- Around line 144-145: Remove the leftover insertion note comment inside the
describe('mergeConfigurations') test suite in src/updates/main.spec.ts; locate
the stray comment text (the "// Add these inside describe('mergeConfigurations')
block" marker) and delete it so the test file contains only the actual tests and
pre-existing comments, keeping the describe('mergeConfigurations') block
self-contained and uncluttered.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fa85cea8-20b5-48c0-94bc-37698f3a14fb

📥 Commits

Reviewing files that changed from the base of the PR and between 49261f5 and 4963be9.

📒 Files selected for processing (3)
  • src/ui/components/DownloadsManagerView/DownloadItem.tsx
  • src/ui/components/UpdateDialog/index.tsx
  • src/updates/main.spec.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use TypeScript strict mode enabled in TypeScript configuration
Use React functional components with hooks instead of class components
Follow FSA (Flux Standard Action) pattern for Redux actions
Use camelCase for file names and PascalCase for component file names
All code must pass ESLint and TypeScript checks
Write self-documenting code with clear naming; avoid unnecessary comments except for complex business logic or non-obvious decisions
Use Fuselage components from @rocket.chat/fuselage for all UI work and only create custom components when Fuselage doesn't provide what's needed
Check Theme.d.ts for valid color tokens when using Fuselage components
Use defensive coding with optional chaining and fallbacks for Linux-only APIs (process.getuid(), process.getgid(), process.geteuid(), process.getegid()) to ensure cross-platform compatibility across Windows, macOS, and Linux

Files:

  • src/ui/components/UpdateDialog/index.tsx
  • src/updates/main.spec.ts
  • src/ui/components/DownloadsManagerView/DownloadItem.tsx
**/*.{spec,main.spec}.ts

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{spec,main.spec}.ts: Use *.spec.ts file naming for renderer process tests and *.main.spec.ts for main process tests
Only mock platform-specific APIs in tests when defensive coding patterns cannot be used; use Object.defineProperty to mock process.platform and environment variables

Files:

  • src/updates/main.spec.ts
🧠 Learnings (2)
📚 Learning: 2026-02-04T19:29:54.650Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-04T19:29:54.650Z
Learning: Applies to **/*.{spec,main.spec}.ts : Use `*.spec.ts` file naming for renderer process tests and `*.main.spec.ts` for main process tests

Applied to files:

  • src/updates/main.spec.ts
📚 Learning: 2026-02-04T19:29:54.650Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.Electron PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-02-04T19:29:54.650Z
Learning: Applies to **/*.{spec,main.spec}.ts : Only mock platform-specific APIs in tests when defensive coding patterns cannot be used; use Object.defineProperty to mock process.platform and environment variables

Applied to files:

  • src/updates/main.spec.ts
🪛 ESLint
src/ui/components/UpdateDialog/index.tsx

[error] 92-92: Replace ·type='button'·onClick={handleSkipButtonClick}·aria-label={t('dialog.update.skip')} with ⏎··········type='button'⏎··········onClick={handleSkipButtonClick}⏎··········aria-label={t('dialog.update.skip')}⏎·······

(prettier/prettier)


[error] 95-95: Replace ·type='button'·onClick={handleRemindLaterButtonClick}·aria-label={t('dialog.update.remindLater')} with ⏎··········type='button'⏎··········onClick={handleRemindLaterButtonClick}⏎··········aria-label={t('dialog.update.remindLater')}⏎·······

(prettier/prettier)


[error] 103-103: Delete ·

(prettier/prettier)

src/updates/main.spec.ts

[error] 144-144: Insert ··

(prettier/prettier)


[error] 146-146: Insert ··

(prettier/prettier)


[error] 147-147: Insert ··

(prettier/prettier)


[error] 148-148: Insert ··

(prettier/prettier)


[error] 149-149: Insert ··

(prettier/prettier)


[error] 150-150: Insert ··

(prettier/prettier)


[error] 151-151: Insert ··

(prettier/prettier)


[error] 152-152: Insert ··

(prettier/prettier)


[error] 153-153: Insert ··

(prettier/prettier)


[error] 154-154: Insert ··

(prettier/prettier)


[error] 155-155: Insert ··

(prettier/prettier)


[error] 156-156: Insert ··

(prettier/prettier)


[error] 157-157: Insert ··

(prettier/prettier)


[error] 158-158: Insert ··

(prettier/prettier)


[error] 159-159: Insert ··

(prettier/prettier)


[error] 160-160: Replace ·· with ····

(prettier/prettier)


[error] 161-161: Insert ··

(prettier/prettier)


[error] 162-162: Replace ·· with ····

(prettier/prettier)


[error] 163-163: Insert ··

(prettier/prettier)


[error] 165-165: Replace ·· with ····

(prettier/prettier)


[error] 166-166: Insert ··

(prettier/prettier)


[error] 167-167: Replace ······ with ········

(prettier/prettier)


[error] 168-168: Insert ··

(prettier/prettier)


[error] 169-169: Replace ······ with ········

(prettier/prettier)


[error] 170-170: Insert ··

(prettier/prettier)


[error] 171-171: Replace ·· with ····

(prettier/prettier)


[error] 172-172: Insert ··

(prettier/prettier)


[error] 173-173: Replace ···· with ······

(prettier/prettier)


[error] 174-174: Insert ··

(prettier/prettier)


[error] 175-175: Insert ··

(prettier/prettier)


[error] 177-177: Insert ··

(prettier/prettier)


[error] 178-178: Insert ··

(prettier/prettier)


[error] 179-179: Insert ··

(prettier/prettier)


[error] 180-180: Insert ··

(prettier/prettier)


[error] 181-181: Insert ··

(prettier/prettier)


[error] 182-182: Insert ··

(prettier/prettier)


[error] 183-183: Insert ··

(prettier/prettier)


[error] 184-184: Insert ··

(prettier/prettier)


[error] 185-185: Insert ··

(prettier/prettier)


[error] 186-186: Insert ··

(prettier/prettier)


[error] 187-187: Insert ··

(prettier/prettier)


[error] 188-188: Insert ··

(prettier/prettier)


[error] 189-189: Insert ··

(prettier/prettier)


[error] 190-190: Insert ··

(prettier/prettier)


[error] 191-191: Insert ··

(prettier/prettier)


[error] 192-192: Insert ··

(prettier/prettier)


[error] 193-193: Replace ···· with ······

(prettier/prettier)


[error] 194-194: Insert ··

(prettier/prettier)


[error] 196-196: Replace ·· with ····

(prettier/prettier)


[error] 197-197: Insert ··

(prettier/prettier)


[error] 198-198: Replace ······ with ········

(prettier/prettier)


[error] 199-199: Insert ··

(prettier/prettier)


[error] 200-200: Replace ······ with ········

(prettier/prettier)


[error] 201-201: Insert ··

(prettier/prettier)


[error] 202-202: Insert ··

(prettier/prettier)


[error] 203-203: Insert ··

(prettier/prettier)


[error] 204-204: Insert ··

(prettier/prettier)


[error] 205-205: Insert ··

(prettier/prettier)


[error] 206-206: Insert ··

(prettier/prettier)


[error] 208-208: Insert ··

(prettier/prettier)


[error] 209-209: Insert ··

(prettier/prettier)


[error] 210-210: Replace ···· with ······

(prettier/prettier)


[error] 211-211: Insert ··

(prettier/prettier)


[error] 212-212: Replace ···· with ······

(prettier/prettier)


[error] 213-213: Insert ··

(prettier/prettier)


[error] 214-214: Replace ···· with ······

(prettier/prettier)


[error] 215-215: Insert ··

(prettier/prettier)


[error] 216-216: Replace ···· with ······

(prettier/prettier)


[error] 217-217: Insert ··

(prettier/prettier)


[error] 218-218: Replace ···· with ······

(prettier/prettier)


[error] 219-219: Insert ··

(prettier/prettier)


[error] 220-220: Replace ···· with ······

(prettier/prettier)


[error] 221-221: Insert ··

(prettier/prettier)


[error] 222-222: Replace ·· with ····

(prettier/prettier)


[error] 223-223: Insert ··

(prettier/prettier)


[error] 224-224: Insert ··

(prettier/prettier)


[error] 225-225: Insert ··

(prettier/prettier)


[error] 226-226: Insert ··

(prettier/prettier)


[error] 227-227: Insert ··

(prettier/prettier)


[error] 228-228: Insert ··

(prettier/prettier)


[error] 230-230: Insert ··

(prettier/prettier)


[error] 231-231: Insert ··

(prettier/prettier)


[error] 232-232: Insert ··

(prettier/prettier)


[error] 233-233: Insert ··

(prettier/prettier)


[error] 234-234: Replace ······ with ········

(prettier/prettier)


[error] 235-235: Insert ··

(prettier/prettier)


[error] 236-236: Replace ·· with ····

(prettier/prettier)


[error] 237-237: Insert ··

(prettier/prettier)


[error] 238-238: Replace ···· with ······

(prettier/prettier)


[error] 239-239: Insert ··

(prettier/prettier)


[error] 240-240: Insert ··

(prettier/prettier)


[error] 241-241: Insert ··

(prettier/prettier)


[error] 243-243: Insert ··

(prettier/prettier)


[error] 244-244: Insert ··

(prettier/prettier)


[error] 245-245: Insert ··

(prettier/prettier)


[error] 246-246: Insert ··

(prettier/prettier)


[error] 247-247: Insert ··

(prettier/prettier)


[error] 248-248: Insert ··

(prettier/prettier)


[error] 249-249: Insert ··

(prettier/prettier)


[error] 250-250: Insert ··

(prettier/prettier)


[error] 251-251: Insert ··

(prettier/prettier)


[error] 252-252: Insert ··

(prettier/prettier)


[error] 253-253: Insert ··

(prettier/prettier)


[error] 254-254: Insert ··

(prettier/prettier)


[error] 255-255: Insert ··

(prettier/prettier)


[error] 256-256: Insert ··

(prettier/prettier)


[error] 257-257: Insert ··

(prettier/prettier)


[error] 258-258: Replace ·· with ····

(prettier/prettier)


[error] 260-260: Insert ··

(prettier/prettier)


[error] 261-261: Replace ···· with ······

(prettier/prettier)


[error] 262-262: Insert ··

(prettier/prettier)


[error] 263-263: Replace ······ with ········

(prettier/prettier)


[error] 264-264: Insert ··

(prettier/prettier)


[error] 265-265: Insert ··

(prettier/prettier)


[error] 266-266: Insert ··

(prettier/prettier)


[error] 267-267: Insert ··

(prettier/prettier)


[error] 268-268: Insert ··

(prettier/prettier)


[error] 269-269: Insert ··

(prettier/prettier)


[error] 270-276: Replace });⏎·⏎⏎⏎⏎⏎ with ··});

(prettier/prettier)

src/ui/components/DownloadsManagerView/DownloadItem.tsx

[error] 185-185: Replace ·onClick={handleRemove}·aria-label={t('downloads.item.remove')} with ⏎················onClick={handleRemove}⏎················aria-label={t('downloads.item.remove')}⏎··············

(prettier/prettier)


[error] 190-190: Replace ·onClick={handleCopyLink}·aria-label={t('downloads.item.copyLink')} with ⏎················onClick={handleCopyLink}⏎················aria-label={t('downloads.item.copyLink')}⏎··············

(prettier/prettier)


[error] 196-196: Replace ·onClick={handlePause}··aria-label={t('downloads.item.pause')} with ⏎··················onClick={handlePause}⏎··················aria-label={t('downloads.item.pause')}⏎················

(prettier/prettier)


[error] 199-199: Replace ·onClick={handleCancel}·aria-label={t('downloads.item.cancel')} with ⏎··················onClick={handleCancel}⏎··················aria-label={t('downloads.item.cancel')}⏎················

(prettier/prettier)


[error] 206-206: Replace ·onClick={handleResume}·aria-label={t('downloads.item.resume')} with ⏎··················onClick={handleResume}⏎··················aria-label={t('downloads.item.resume')}⏎················

(prettier/prettier)


[error] 209-209: Replace ·onClick={handleCancel}·aria-label={t('downloads.item.cancel')} with ⏎··················onClick={handleCancel}⏎··················aria-label={t('downloads.item.cancel')}⏎················

(prettier/prettier)


[error] 216-216: Replace ·onClick={handleShowInFolder}··aria-label={t('downloads.item.showInFolder')} with ⏎··················onClick={handleShowInFolder}⏎··················aria-label={t('downloads.item.showInFolder')}⏎················

(prettier/prettier)


[error] 219-219: Replace ·onClick={handleRemove}·aria-label={t('downloads.item.remove')} with ⏎··················onClick={handleRemove}⏎··················aria-label={t('downloads.item.remove')}⏎················

(prettier/prettier)


[error] 226-226: Replace ·onClick={handleRetry}·aria-label={t('downloads.item.retry')} with ⏎··················onClick={handleRetry}⏎··················aria-label={t('downloads.item.retry')}⏎················

(prettier/prettier)


[error] 229-229: Replace ·onClick={handleRemove}·aria-label={t('downloads.item.remove')} with ⏎··················onClick={handleRemove}⏎··················aria-label={t('downloads.item.remove')}⏎················

(prettier/prettier)

Comment on lines +185 to +229
<ActionButton onClick={handleRemove} aria-label={t('downloads.item.remove')}>
{t('downloads.item.remove')}
</ActionButton>
)}
{!expired && (
<ActionButton onClick={handleCopyLink}>
<ActionButton onClick={handleCopyLink} aria-label={t('downloads.item.copyLink')}>
{t('downloads.item.copyLink')}
</ActionButton>
)}
{state === 'progressing' && (
<>
<ActionButton onClick={handlePause}>
<ActionButton onClick={handlePause} aria-label={t('downloads.item.pause')}>
{t('downloads.item.pause')}
</ActionButton>
<ActionButton onClick={handleCancel}>
<ActionButton onClick={handleCancel} aria-label={t('downloads.item.cancel')}>
{t('downloads.item.cancel')}
</ActionButton>
</>
)}
{state === 'paused' && (
<>
<ActionButton onClick={handleResume}>
<ActionButton onClick={handleResume} aria-label={t('downloads.item.resume')}>
{t('downloads.item.resume')}
</ActionButton>
<ActionButton onClick={handleCancel}>
<ActionButton onClick={handleCancel} aria-label={t('downloads.item.cancel')}>
{t('downloads.item.cancel')}
</ActionButton>
</>
)}
{state === 'completed' && (
<>
<ActionButton onClick={handleShowInFolder}>
<ActionButton onClick={handleShowInFolder} aria-label={t('downloads.item.showInFolder')}>
{t('downloads.item.showInFolder')}
</ActionButton>
<ActionButton onClick={handleRemove}>
<ActionButton onClick={handleRemove} aria-label={t('downloads.item.remove')}>
{t('downloads.item.remove')}
</ActionButton>
</>
)}
{errored && (
<>
<ActionButton onClick={handleRetry}>
<ActionButton onClick={handleRetry} aria-label={t('downloads.item.retry')}>
{t('downloads.item.retry')}
</ActionButton>
<ActionButton onClick={handleRemove}>
<ActionButton onClick={handleRemove} aria-label={t('downloads.item.remove')}>
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Run Prettier on this JSX block before merge.

The new aria-label props look good, but this hunk is currently failing prettier/prettier, so it will not pass lint as committed.

As per coding guidelines, "All code must pass ESLint and TypeScript checks".

🧰 Tools
🪛 ESLint

[error] 185-185: Replace ·onClick={handleRemove}·aria-label={t('downloads.item.remove')} with ⏎················onClick={handleRemove}⏎················aria-label={t('downloads.item.remove')}⏎··············

(prettier/prettier)


[error] 190-190: Replace ·onClick={handleCopyLink}·aria-label={t('downloads.item.copyLink')} with ⏎················onClick={handleCopyLink}⏎················aria-label={t('downloads.item.copyLink')}⏎··············

(prettier/prettier)


[error] 196-196: Replace ·onClick={handlePause}··aria-label={t('downloads.item.pause')} with ⏎··················onClick={handlePause}⏎··················aria-label={t('downloads.item.pause')}⏎················

(prettier/prettier)


[error] 199-199: Replace ·onClick={handleCancel}·aria-label={t('downloads.item.cancel')} with ⏎··················onClick={handleCancel}⏎··················aria-label={t('downloads.item.cancel')}⏎················

(prettier/prettier)


[error] 206-206: Replace ·onClick={handleResume}·aria-label={t('downloads.item.resume')} with ⏎··················onClick={handleResume}⏎··················aria-label={t('downloads.item.resume')}⏎················

(prettier/prettier)


[error] 209-209: Replace ·onClick={handleCancel}·aria-label={t('downloads.item.cancel')} with ⏎··················onClick={handleCancel}⏎··················aria-label={t('downloads.item.cancel')}⏎················

(prettier/prettier)


[error] 216-216: Replace ·onClick={handleShowInFolder}··aria-label={t('downloads.item.showInFolder')} with ⏎··················onClick={handleShowInFolder}⏎··················aria-label={t('downloads.item.showInFolder')}⏎················

(prettier/prettier)


[error] 219-219: Replace ·onClick={handleRemove}·aria-label={t('downloads.item.remove')} with ⏎··················onClick={handleRemove}⏎··················aria-label={t('downloads.item.remove')}⏎················

(prettier/prettier)


[error] 226-226: Replace ·onClick={handleRetry}·aria-label={t('downloads.item.retry')} with ⏎··················onClick={handleRetry}⏎··················aria-label={t('downloads.item.retry')}⏎················

(prettier/prettier)


[error] 229-229: Replace ·onClick={handleRemove}·aria-label={t('downloads.item.remove')} with ⏎··················onClick={handleRemove}⏎··················aria-label={t('downloads.item.remove')}⏎················

(prettier/prettier)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/ui/components/DownloadsManagerView/DownloadItem.tsx` around lines 185 -
229, The JSX block using ActionButton (handlers: handleRemove, handleCopyLink,
handlePause, handleCancel, handleResume, handleShowInFolder, handleRetry) is
failing prettier; run Prettier (or your project's format script) on this
component (DownloadItem.tsx) to reformat the ActionButton elements and their
aria-label props consistently so the file passes prettier/prettier and
ESLint/TypeScript checks, then re-commit the formatted file.

Comment on lines +92 to +103
<Button type='button' onClick={handleSkipButtonClick} aria-label={t('dialog.update.skip')} >
{t('dialog.update.skip')}
</Button>
<Button type='button' onClick={handleRemindLaterButtonClick}>
<Button type='button' onClick={handleRemindLaterButtonClick} aria-label={t('dialog.update.remindLater')} >
{t('dialog.update.remindLater')}
</Button>
<Button
ref={installButtonRef}
type='button'
primary
onClick={handleInstallButtonClick}
aria-label={t('dialog.update.install')}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

This button hunk still needs formatting cleanup.

Prettier is failing on the added props here, including the extra whitespace before aria-label on the install button.

As per coding guidelines, "All code must pass ESLint and TypeScript checks".

🧰 Tools
🪛 ESLint

[error] 92-92: Replace ·type='button'·onClick={handleSkipButtonClick}·aria-label={t('dialog.update.skip')} with ⏎··········type='button'⏎··········onClick={handleSkipButtonClick}⏎··········aria-label={t('dialog.update.skip')}⏎·······

(prettier/prettier)


[error] 95-95: Replace ·type='button'·onClick={handleRemindLaterButtonClick}·aria-label={t('dialog.update.remindLater')} with ⏎··········type='button'⏎··········onClick={handleRemindLaterButtonClick}⏎··········aria-label={t('dialog.update.remindLater')}⏎·······

(prettier/prettier)


[error] 103-103: Delete ·

(prettier/prettier)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/ui/components/UpdateDialog/index.tsx` around lines 92 - 103, The Button
prop formatting is inconsistent (extra whitespace) and breaks Prettier; fix the
three Button JSX elements so props are consistently ordered and spaced and the
install Button's attributes are on one line or following project JSX wrapping
rules; specifically adjust the Button elements where onClick handlers call
handleSkipButtonClick, handleRemindLaterButtonClick, and the install Button that
uses ref={installButtonRef} and onClick={handleInstallButtonClick} to remove the
stray space before aria-label and ensure proper indentation/wrapping so
ESLint/Prettier/TypeScript all pass.

Comment on lines +146 to +270
it('merges app channel configuration', () => {
const defaultConfiguration: UpdateConfiguration = {
doCheckForUpdatesOnStartup: true,
isEachUpdatesSettingConfigurable: true,
isUpdatingAllowed: true,
isUpdatingEnabled: true,
skippedUpdateVersion: null,
isReportEnabled: true,
isFlashFrameEnabled: true,
isHardwareAccelerationEnabled: true,
isInternalVideoChatWindowEnabled: true,
isVideoCallScreenCaptureFallbackEnabled: false,
updateChannel: 'latest',
};
const appConfiguration: AppLevelUpdateConfiguration = {
channel: 'beta',
};
const userConfiguration: UserLevelUpdateConfiguration = {};

expect(
mergeConfigurations(
defaultConfiguration,
appConfiguration,
userConfiguration
)
).toStrictEqual({
...defaultConfiguration,
updateChannel: 'beta',
});
});

it('user channel overrides default when configurable', () => {
const defaultConfiguration: UpdateConfiguration = {
doCheckForUpdatesOnStartup: true,
isEachUpdatesSettingConfigurable: true,
isUpdatingAllowed: true,
isUpdatingEnabled: true,
skippedUpdateVersion: null,
isReportEnabled: true,
isFlashFrameEnabled: true,
isHardwareAccelerationEnabled: true,
isInternalVideoChatWindowEnabled: true,
isVideoCallScreenCaptureFallbackEnabled: false,
updateChannel: 'latest',
};
const appConfiguration: AppLevelUpdateConfiguration = {};
const userConfiguration: UserLevelUpdateConfiguration = {
channel: 'alpha',
};

expect(
mergeConfigurations(
defaultConfiguration,
appConfiguration,
userConfiguration
)
).toStrictEqual({
...defaultConfiguration,
updateChannel: 'alpha',
});
});

it('forced app config blocks user channel override', () => {
const defaultConfiguration: UpdateConfiguration = {
doCheckForUpdatesOnStartup: true,
isEachUpdatesSettingConfigurable: true,
isUpdatingAllowed: true,
isUpdatingEnabled: true,
skippedUpdateVersion: null,
isReportEnabled: true,
isFlashFrameEnabled: true,
isHardwareAccelerationEnabled: true,
isInternalVideoChatWindowEnabled: true,
isVideoCallScreenCaptureFallbackEnabled: false,
updateChannel: 'latest',
};
const appConfiguration: AppLevelUpdateConfiguration = {
forced: true,
channel: 'beta',
};
const userConfiguration: UserLevelUpdateConfiguration = {
channel: 'alpha',
};

expect(
mergeConfigurations(
defaultConfiguration,
appConfiguration,
userConfiguration
)
).toStrictEqual({
...defaultConfiguration,
isEachUpdatesSettingConfigurable: false,
updateChannel: 'beta', // user alpha ignored because forced
});
});

it('handles null skip value in user configuration', () => {
const defaultConfiguration: UpdateConfiguration = {
doCheckForUpdatesOnStartup: true,
isEachUpdatesSettingConfigurable: true,
isUpdatingAllowed: true,
isUpdatingEnabled: true,
skippedUpdateVersion: '1.0.0',
isReportEnabled: true,
isFlashFrameEnabled: true,
isHardwareAccelerationEnabled: true,
isInternalVideoChatWindowEnabled: true,
isVideoCallScreenCaptureFallbackEnabled: false,
updateChannel: 'latest',
};
const appConfiguration: AppLevelUpdateConfiguration = {};
const userConfiguration: UserLevelUpdateConfiguration = {};

expect(
mergeConfigurations(
defaultConfiguration,
appConfiguration,
userConfiguration
)
).toStrictEqual({
...defaultConfiguration,
skippedUpdateVersion: '1.0.0', // unchanged
});
});
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Format the new test block before landing it.

This entire insertion is currently out of sync with Prettier indentation, so the spec file will fail lint until it is reformatted.

As per coding guidelines, "All code must pass ESLint and TypeScript checks".

🧰 Tools
🪛 ESLint

[error] 146-146: Insert ··

(prettier/prettier)


[error] 147-147: Insert ··

(prettier/prettier)


[error] 148-148: Insert ··

(prettier/prettier)


[error] 149-149: Insert ··

(prettier/prettier)


[error] 150-150: Insert ··

(prettier/prettier)


[error] 151-151: Insert ··

(prettier/prettier)


[error] 152-152: Insert ··

(prettier/prettier)


[error] 153-153: Insert ··

(prettier/prettier)


[error] 154-154: Insert ··

(prettier/prettier)


[error] 155-155: Insert ··

(prettier/prettier)


[error] 156-156: Insert ··

(prettier/prettier)


[error] 157-157: Insert ··

(prettier/prettier)


[error] 158-158: Insert ··

(prettier/prettier)


[error] 159-159: Insert ··

(prettier/prettier)


[error] 160-160: Replace ·· with ····

(prettier/prettier)


[error] 161-161: Insert ··

(prettier/prettier)


[error] 162-162: Replace ·· with ····

(prettier/prettier)


[error] 163-163: Insert ··

(prettier/prettier)


[error] 165-165: Replace ·· with ····

(prettier/prettier)


[error] 166-166: Insert ··

(prettier/prettier)


[error] 167-167: Replace ······ with ········

(prettier/prettier)


[error] 168-168: Insert ··

(prettier/prettier)


[error] 169-169: Replace ······ with ········

(prettier/prettier)


[error] 170-170: Insert ··

(prettier/prettier)


[error] 171-171: Replace ·· with ····

(prettier/prettier)


[error] 172-172: Insert ··

(prettier/prettier)


[error] 173-173: Replace ···· with ······

(prettier/prettier)


[error] 174-174: Insert ··

(prettier/prettier)


[error] 175-175: Insert ··

(prettier/prettier)


[error] 177-177: Insert ··

(prettier/prettier)


[error] 178-178: Insert ··

(prettier/prettier)


[error] 179-179: Insert ··

(prettier/prettier)


[error] 180-180: Insert ··

(prettier/prettier)


[error] 181-181: Insert ··

(prettier/prettier)


[error] 182-182: Insert ··

(prettier/prettier)


[error] 183-183: Insert ··

(prettier/prettier)


[error] 184-184: Insert ··

(prettier/prettier)


[error] 185-185: Insert ··

(prettier/prettier)


[error] 186-186: Insert ··

(prettier/prettier)


[error] 187-187: Insert ··

(prettier/prettier)


[error] 188-188: Insert ··

(prettier/prettier)


[error] 189-189: Insert ··

(prettier/prettier)


[error] 190-190: Insert ··

(prettier/prettier)


[error] 191-191: Insert ··

(prettier/prettier)


[error] 192-192: Insert ··

(prettier/prettier)


[error] 193-193: Replace ···· with ······

(prettier/prettier)


[error] 194-194: Insert ··

(prettier/prettier)


[error] 196-196: Replace ·· with ····

(prettier/prettier)


[error] 197-197: Insert ··

(prettier/prettier)


[error] 198-198: Replace ······ with ········

(prettier/prettier)


[error] 199-199: Insert ··

(prettier/prettier)


[error] 200-200: Replace ······ with ········

(prettier/prettier)


[error] 201-201: Insert ··

(prettier/prettier)


[error] 202-202: Insert ··

(prettier/prettier)


[error] 203-203: Insert ··

(prettier/prettier)


[error] 204-204: Insert ··

(prettier/prettier)


[error] 205-205: Insert ··

(prettier/prettier)


[error] 206-206: Insert ··

(prettier/prettier)


[error] 208-208: Insert ··

(prettier/prettier)


[error] 209-209: Insert ··

(prettier/prettier)


[error] 210-210: Replace ···· with ······

(prettier/prettier)


[error] 211-211: Insert ··

(prettier/prettier)


[error] 212-212: Replace ···· with ······

(prettier/prettier)


[error] 213-213: Insert ··

(prettier/prettier)


[error] 214-214: Replace ···· with ······

(prettier/prettier)


[error] 215-215: Insert ··

(prettier/prettier)


[error] 216-216: Replace ···· with ······

(prettier/prettier)


[error] 217-217: Insert ··

(prettier/prettier)


[error] 218-218: Replace ···· with ······

(prettier/prettier)


[error] 219-219: Insert ··

(prettier/prettier)


[error] 220-220: Replace ···· with ······

(prettier/prettier)


[error] 221-221: Insert ··

(prettier/prettier)


[error] 222-222: Replace ·· with ····

(prettier/prettier)


[error] 223-223: Insert ··

(prettier/prettier)


[error] 224-224: Insert ··

(prettier/prettier)


[error] 225-225: Insert ··

(prettier/prettier)


[error] 226-226: Insert ··

(prettier/prettier)


[error] 227-227: Insert ··

(prettier/prettier)


[error] 228-228: Insert ··

(prettier/prettier)


[error] 230-230: Insert ··

(prettier/prettier)


[error] 231-231: Insert ··

(prettier/prettier)


[error] 232-232: Insert ··

(prettier/prettier)


[error] 233-233: Insert ··

(prettier/prettier)


[error] 234-234: Replace ······ with ········

(prettier/prettier)


[error] 235-235: Insert ··

(prettier/prettier)


[error] 236-236: Replace ·· with ····

(prettier/prettier)


[error] 237-237: Insert ··

(prettier/prettier)


[error] 238-238: Replace ···· with ······

(prettier/prettier)


[error] 239-239: Insert ··

(prettier/prettier)


[error] 240-240: Insert ··

(prettier/prettier)


[error] 241-241: Insert ··

(prettier/prettier)


[error] 243-243: Insert ··

(prettier/prettier)


[error] 244-244: Insert ··

(prettier/prettier)


[error] 245-245: Insert ··

(prettier/prettier)


[error] 246-246: Insert ··

(prettier/prettier)


[error] 247-247: Insert ··

(prettier/prettier)


[error] 248-248: Insert ··

(prettier/prettier)


[error] 249-249: Insert ··

(prettier/prettier)


[error] 250-250: Insert ··

(prettier/prettier)


[error] 251-251: Insert ··

(prettier/prettier)


[error] 252-252: Insert ··

(prettier/prettier)


[error] 253-253: Insert ··

(prettier/prettier)


[error] 254-254: Insert ··

(prettier/prettier)


[error] 255-255: Insert ··

(prettier/prettier)


[error] 256-256: Insert ··

(prettier/prettier)


[error] 257-257: Insert ··

(prettier/prettier)


[error] 258-258: Replace ·· with ····

(prettier/prettier)


[error] 260-260: Insert ··

(prettier/prettier)


[error] 261-261: Replace ···· with ······

(prettier/prettier)


[error] 262-262: Insert ··

(prettier/prettier)


[error] 263-263: Replace ······ with ········

(prettier/prettier)


[error] 264-264: Insert ··

(prettier/prettier)


[error] 265-265: Insert ··

(prettier/prettier)


[error] 266-266: Insert ··

(prettier/prettier)


[error] 267-267: Insert ··

(prettier/prettier)


[error] 268-268: Insert ··

(prettier/prettier)


[error] 269-269: Insert ··

(prettier/prettier)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/updates/main.spec.ts` around lines 146 - 270, The newly added test block
in src/updates/main.spec.ts is misformatted and violates Prettier/ESLint rules;
run the project formatter (prettier) or your editor's format command on the file
and fix indentation/spacing for the inserted tests (those new it(...) blocks
that use mergeConfigurations) so the file passes lint/TS checks; ensure
consistent indentation, trailing commas and alignment with existing tests, then
re-run lint and unit tests before pushing.

Comment on lines +243 to +258
it('handles null skip value in user configuration', () => {
const defaultConfiguration: UpdateConfiguration = {
doCheckForUpdatesOnStartup: true,
isEachUpdatesSettingConfigurable: true,
isUpdatingAllowed: true,
isUpdatingEnabled: true,
skippedUpdateVersion: '1.0.0',
isReportEnabled: true,
isFlashFrameEnabled: true,
isHardwareAccelerationEnabled: true,
isInternalVideoChatWindowEnabled: true,
isVideoCallScreenCaptureFallbackEnabled: false,
updateChannel: 'latest',
};
const appConfiguration: AppLevelUpdateConfiguration = {};
const userConfiguration: UserLevelUpdateConfiguration = {};
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

The test name doesn't match the scenario being exercised.

This setup covers "skip omitted", not "skip is null", because userConfiguration is {}. Either rename the test to reflect the omitted case, or set skip: null so the explicit null path is actually covered.

One possible fix
-it('handles null skip value in user configuration', () => {
+it('preserves skippedUpdateVersion when user skip is omitted', () => {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
it('handles null skip value in user configuration', () => {
const defaultConfiguration: UpdateConfiguration = {
doCheckForUpdatesOnStartup: true,
isEachUpdatesSettingConfigurable: true,
isUpdatingAllowed: true,
isUpdatingEnabled: true,
skippedUpdateVersion: '1.0.0',
isReportEnabled: true,
isFlashFrameEnabled: true,
isHardwareAccelerationEnabled: true,
isInternalVideoChatWindowEnabled: true,
isVideoCallScreenCaptureFallbackEnabled: false,
updateChannel: 'latest',
};
const appConfiguration: AppLevelUpdateConfiguration = {};
const userConfiguration: UserLevelUpdateConfiguration = {};
it('preserves skippedUpdateVersion when user skip is omitted', () => {
const defaultConfiguration: UpdateConfiguration = {
doCheckForUpdatesOnStartup: true,
isEachUpdatesSettingConfigurable: true,
isUpdatingAllowed: true,
isUpdatingEnabled: true,
skippedUpdateVersion: '1.0.0',
isReportEnabled: true,
isFlashFrameEnabled: true,
isHardwareAccelerationEnabled: true,
isInternalVideoChatWindowEnabled: true,
isVideoCallScreenCaptureFallbackEnabled: false,
updateChannel: 'latest',
};
const appConfiguration: AppLevelUpdateConfiguration = {};
const userConfiguration: UserLevelUpdateConfiguration = {};
🧰 Tools
🪛 ESLint

[error] 243-243: Insert ··

(prettier/prettier)


[error] 244-244: Insert ··

(prettier/prettier)


[error] 245-245: Insert ··

(prettier/prettier)


[error] 246-246: Insert ··

(prettier/prettier)


[error] 247-247: Insert ··

(prettier/prettier)


[error] 248-248: Insert ··

(prettier/prettier)


[error] 249-249: Insert ··

(prettier/prettier)


[error] 250-250: Insert ··

(prettier/prettier)


[error] 251-251: Insert ··

(prettier/prettier)


[error] 252-252: Insert ··

(prettier/prettier)


[error] 253-253: Insert ··

(prettier/prettier)


[error] 254-254: Insert ··

(prettier/prettier)


[error] 255-255: Insert ··

(prettier/prettier)


[error] 256-256: Insert ··

(prettier/prettier)


[error] 257-257: Insert ··

(prettier/prettier)


[error] 258-258: Replace ·· with ····

(prettier/prettier)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/updates/main.spec.ts` around lines 243 - 258, The test titled "handles
null skip value in user configuration" is actually exercising the omitted-skip
case because userConfiguration is an empty object; update the test to match the
scenario by either renaming the test to reflect "skip omitted" or modify the
setup so userConfiguration explicitly sets skippedUpdateVersion: null to cover
the null-path; adjust the assertion expectations accordingly and reference the
test name string and the variables defaultConfiguration, appConfiguration, and
userConfiguration when making the change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant