Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/locales/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@
"pad.modals.disconnected": "You have been disconnected.",
"pad.modals.disconnected.explanation": "The connection to the server was lost",
"pad.modals.disconnected.cause": "The server may be unavailable. Please notify the service administrator if this continues to happen.",
"pad.gritter.unacceptedCommit.title": "Unsaved edit",
"pad.gritter.unacceptedCommit.text": "Your recent edit is still not saved. Reconnect and try again.",

"pad.share": "Share this pad",
"pad.share.readonly": "Read only",
Expand Down
2 changes: 2 additions & 0 deletions src/static/js/collab_client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ const getCollabClient = (ace2editor, serverVars, initialUserInfo, options, _pad)

const acceptCommit = () => {
editor.applyPreparedChangesetToBase();
stateMessage = null;
setStateIdle();
try {
callbacks.onInternalAction('commitAcceptedByServer');
Expand Down Expand Up @@ -488,6 +489,7 @@ const getCollabClient = (ace2editor, serverVars, initialUserInfo, options, _pad)
sendMessage,
getCurrentRevisionNumber,
getMissedChanges,
hasUnacceptedCommit: () => stateMessage != null,
callWhenNotCommitting,
addHistoricalAuthors: tellAceAboutHistoricalAuthors,
setChannelState,
Expand Down
9 changes: 9 additions & 0 deletions src/static/js/pad.ts
Original file line number Diff line number Diff line change
Expand Up @@ -655,6 +655,14 @@ const pad = {
pad.handleOptionsChange(opts);
}
},
showUnacceptedCommitWarning: () => {
$.gritter.add({
title: html10n.get('pad.gritter.unacceptedCommit.title'),
text: html10n.get('pad.gritter.unacceptedCommit.text'),
sticky: true,
class_name: 'disconnected unsaved-warning',
});
},
Comment on lines +658 to +665
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

2. No test for unaccepted warning 📘 Rule violation ☼ Reliability

This bug fix changes client behavior around pending commits and disconnect warnings but does not
include any regression test in the change set. Without an automated test, the issue can reappear
undetected if the fix is reverted or refactored.
Agent Prompt
## Issue description
The PR changes client behavior for pending commits/disconnect warnings but does not add a regression test that would fail without the fix.

## Issue Context
A suitable regression test should assert at least one of:
- `stateMessage`/pending marker is cleared when `ACCEPT_COMMIT` is processed (so `hasUnacceptedCommit()` flips to `false`).
- The warning UI is triggered when a disconnect occurs while a commit is pending.

## Fix Focus Areas
- src/static/js/collab_client.ts[142-151]
- src/static/js/collab_client.ts[492-500]
- src/static/js/pad.ts[658-665]
- src/static/js/pad.ts[703-703]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

handleChannelStateChange: (newState, message) => {
const oldFullyConnected = !!padconnectionstatus.isFullyConnected();
const wasConnecting = (padconnectionstatus.getStatus().what === 'connecting');
Expand Down Expand Up @@ -692,6 +700,7 @@ const pad = {
padimpexp.disable();

padconnectionstatus.disconnected(message);
if (pad.collabClient.hasUnacceptedCommit()) pad.showUnacceptedCommitWarning();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Pending edits only warned on disconnect 📎 Requirement gap ☼ Reliability

The new UX warning for unaccepted commits is only shown when the channel enters DISCONNECTED, so
users can still have locally-applied edits pending acceptance with no in-UI indication during the
pending state. This fails the requirement to indicate the pending state itself (and clear/update
once accepted).
Agent Prompt
## Issue description
The UI warning for unaccepted commits is only displayed upon disconnect, not throughout the entire pending-acceptance window. Compliance requires a clear indication whenever a commit is pending (until `ACCEPT_COMMIT` clears it).

## Issue Context
A pending commit is tracked via `stateMessage` and exposed as `hasUnacceptedCommit()`, and `stateMessage` is cleared in `acceptCommit()`. However, the UI only calls `showUnacceptedCommitWarning()` inside the `DISCONNECTED` channel state, so users may still believe edits are saved while connected.

## Fix Focus Areas
- src/static/js/pad.ts[658-665]
- src/static/js/pad.ts[703-703]
- src/static/js/collab_client.ts[142-151]
- src/static/js/collab_client.ts[492-500]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

}
const newFullyConnected = !!padconnectionstatus.isFullyConnected();
if (newFullyConnected !== oldFullyConnected) {
Expand Down
39 changes: 39 additions & 0 deletions src/tests/frontend-new/specs/unaccepted_commit_warning.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import {expect, test} from '@playwright/test';
import {clearPadContent, goToNewPad, writeToPad} from '../helper/padHelper';

test.describe('unaccepted commit warning', () => {
test('hasUnacceptedCommit clears once the server acknowledges the commit',
async ({page}) => {
await goToNewPad(page);
await clearPadContent(page);
await writeToPad(page, 'trigger a commit');

// Wait for the commit to round-trip. The fix clears the pending marker inside
// acceptCommit(); without it the boolean stays true indefinitely.
await expect.poll(async () => await page.evaluate(() =>
(window as any).pad?.collabClient?.hasUnacceptedCommit?.() ?? null,
), {timeout: 10000}).toBe(false);
});

test('disconnect with a pending commit surfaces the unsaved-edit gritter',
async ({page}) => {
await goToNewPad(page);
await page.waitForFunction(() => (window as any).pad?.collabClient != null);

await page.evaluate(() => {
const p: any = (window as any).pad;
// Force the pending-commit predicate to true and simulate a disconnect so
// the warning code path executes deterministically.
p.collabClient.hasUnacceptedCommit = () => true;
p.handleChannelStateChange('DISCONNECTED', {
type: 'disconnect',
explanation: 'test',
cause: 'test',
forIE: false,
canRetry: false,
});
});

await expect(page.locator('.unsaved-warning').first()).toBeVisible();
});
});
Loading