Mirrors/super-productivity
1
0
Fork 0
mirror of https://github.com/johannesjo/super-productivity.git synced 2026-07-17 16:37:43 +00:00
Code Issues Projects Releases Wiki Activity

fix(sync): name the discarded title in LWW conflict banner + fix fr dismiss label (#8694) (#8724)

Browse source 
* ci(release): explain App Store 'previous version in review' failures

When a release is tagged before the previous version clears Apple review,
App Store Connect refuses to create the version or attach the build, and
the lane died with a raw spaceship stacktrace ('cannot create a new
version...' / 'pre-release build could not be added'). Both hit v18.13.1
(iOS + macOS) and v18.12.1 (macOS).

Match those two messages and replace the stacktrace with a one-line,
actionable annotation (let the previous version finish review, or pull it
from review, then re-run) -- still a hard failure, since the version did
not go out and a green run would falsely read as 'released'. The existing
'review submission already in progress' race stays a soft success.

Static annotation text only (no e/options interpolation; key-material).

* fix(sync): name discarded title in LWW conflict banner; fix fr dismiss label (#8694)

* fix(sync): surface the last discarded title in LWW conflict banner

Multi-review follow-up to the #8694 banner:
- Show the LAST non-empty discarded title (the user's final offline rename),
  not the first — a stale intermediate rename is misleading. Also simpler
  (drops the first-wins guard).
- Document why the kept==discarded equality guard is correct: when a title
  edit loses to a concurrent other-field remote win, the current state still
  shows the rejected local title, so an annotation would only repeat it —
  silencing it is right, not a hidden divergence.

fr.json G.DISMISS "Rejeter"->"Ignorer" (prior commit) is an intentional,
owner-approved deviation from the en-only rule: it is a mistranslation fix
for the shared banner dismiss label, not a new string. Other locales may
carry the same "Reject"-flavored label — left as a follow-up.
This commit is contained in:
Johannes Millan 2026-07-03 14:13:43 +02:00 • committed by GitHub
parent 8003a9e125
commit cbeb7c704f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 305 additions and 26 deletions
Download patch file Download diff file Expand all files Collapse all files

66
fastlane/Fastfile
View file

@ -99,22 +99,64 @@ end
# direction.
REVIEW_SUBMISSION_RACE = 'review submission is already in progress'
# App Store Connect also serializes review state per app across releases: while
# one version is still in review, the next version can neither be created nor
# have a build attached. So a release tagged before the previous one clears
# review fails either at version creation ("You cannot create a new version of
# the App in the current state") or at build-attach ("The specified pre-release
# build could not be added"). Unlike the race above this is NOT softened -- the
# version genuinely did not go out, and a green run would falsely read as
# "released". We only trade the raw spaceship stacktrace for a one-line,
# actionable explanation; the lane still fails. Message-matched against fastlane
# deliver 2.225.0 -- if the phrasing drifts it simply falls back to the raw
# error, which is fine (the job still fails).
VERSION_IN_REVIEW_BLOCKS = [
'cannot create a new version',
'pre-release build could not be added',
].freeze
def submit_to_app_store(extra)
upload_to_app_store(deliver_options(extra))
rescue => e
raise unless submit_for_review? && e.message.to_s.downcase.include?(REVIEW_SUBMISSION_RACE)
# Surface as a GitHub Actions run annotation, not just a buried log line, so a
# green job still flags the required manual step. Static text only -- never
# interpolate e/options here (they can carry API key material; see header).
if ENV['GITHUB_ACTIONS']
puts '::warning title=App Store build uploaded but NOT submitted::A review ' \
'submission is already open for this app. Add this build to it in App ' \
'Store Connect, or this platform skips this version.'
msg = e.message.to_s.downcase
# Soft success: the other platform's release lane already opened the shared
# per-app review submission. The binary is uploaded and safe; it just needs to
# be added to the open submission by hand. Surface as a GitHub Actions run
# annotation, not just a buried log line, so a green job still flags the
# required manual step. Static text only -- never interpolate e/options here
# (they can carry API key material; see header).
if submit_for_review? && msg.include?(REVIEW_SUBMISSION_RACE)
if ENV['GITHUB_ACTIONS']
puts '::warning title=App Store build uploaded but NOT submitted::A review ' \
'submission is already open for this app. Add this build to it in App ' \
'Store Connect, or this platform skips this version.'
end
UI.important('Build uploaded, but a review submission is already open for this app')
UI.important("(the other platform's release lane created it first). Add this build to")
UI.important('the open submission in App Store Connect for this version, or this platform')
UI.important('will skip this version. Not failing the lane: the binary is uploaded.')
return
end
UI.important('Build uploaded, but a review submission is already open for this app')
UI.important("(the other platform's release lane created it first). Add this build to")
UI.important('the open submission in App Store Connect for this version, or this platform')
UI.important('will skip this version. Not failing the lane: the binary is uploaded.')
# Known "a previous version is still in review" block. Still a hard failure --
# we only replace the raw stacktrace with a clear, actionable one-liner (the
# raw error still prints after the re-raise). Static text only, same
# key-material reason as above.
if submit_for_review? && VERSION_IN_REVIEW_BLOCKS.any? { |m| msg.include?(m) }
if ENV['GITHUB_ACTIONS']
puts '::error title=App Store submit blocked: previous version still in review::' \
'App Store Connect will not create or submit this version while the ' \
'previous version is still in review. Let it finish review (or remove ' \
'it from review in App Store Connect), then re-run this release.'
end
UI.error('App Store submit blocked: a previous version is still in review.')
UI.error('App Store Connect will not create or submit this version until the')
UI.error('previous one clears review. Let it finish (or pull it from review in')
UI.error('App Store Connect), then re-run this release.')
end
raise
end
platform :ios do

91
src/app/op-log/sync/conflict-resolution.service.spec.ts
View file

@ -551,6 +551,97 @@ describe('ConflictResolutionService', () => {
expect(taskList).toContain('<img');
});
it('names the discarded title when the title itself conflicted (#8694 review)', async () => {
const bannerService = TestBed.inject(BannerService);
const openBannerSpy = spyOn(bannerService, 'open');
// Kept (current) title comes from the store = the winning value; the
// discarded value must still be surfaced so double-check is actionable.
mockStore.select.and.returnValue(of({ title: 'Remote title' }));
const now = Date.now();
const conflicts: EntityConflict[] = [
createConflict(
'task-1',
[
{
...createOpWithTimestamp('local-1', 'client-a', now - 1000),
payload: {
actionPayload: {
task: { id: 'task-1', changes: { title: 'My local title' } },
},
entityChanges: [],
},
},
],
[
{
...createOpWithTimestamp('remote-1', 'client-b', now),
payload: {
actionPayload: {
task: { id: 'task-1', changes: { title: 'Remote title' } },
},
entityChanges: [],
},
},
],
),
];
mockOperationApplier.applyOperations.and.resolveTo({
appliedOps: conflicts[0].remoteOps,
});
await service.autoResolveConflictsLWW(conflicts);
const taskList = openBannerSpy.calls.mostRecent().args[0].translateParams
?.taskList as string;
expect(taskList).toContain('"Remote title"');
expect(taskList).toContain('"My local title"');
});
it('escapes the discarded title too (XSS guard on the new field)', async () => {
const bannerService = TestBed.inject(BannerService);
const openBannerSpy = spyOn(bannerService, 'open');
mockStore.select.and.returnValue(of({ title: 'Kept' }));
const now = Date.now();
const conflicts: EntityConflict[] = [
createConflict(
'task-1',
[
{
...createOpWithTimestamp('local-1', 'client-a', now - 1000),
payload: {
actionPayload: {
task: {
id: 'task-1',
changes: { title: '<img src=x onerror=alert(1)>' },
},
},
entityChanges: [],
},
},
],
[
{
...createOpWithTimestamp('remote-1', 'client-b', now),
payload: {
actionPayload: { task: { id: 'task-1', changes: { title: 'Kept' } } },
entityChanges: [],
},
},
],
),
];
mockOperationApplier.applyOperations.and.resolveTo({
appliedOps: conflicts[0].remoteOps,
});
await service.autoResolveConflictsLWW(conflicts);
const taskList = openBannerSpy.calls.mostRecent().args[0].translateParams
?.taskList as string;
expect(taskList).not.toContain('<img');
expect(taskList).toContain('&lt;img');
});
it('keeps the quiet count snack (no banner) for routine-only resolutions (#8694)', async () => {
const bannerService = TestBed.inject(BannerService);
const openBannerSpy = spyOn(bannerService, 'open');

42
src/app/op-log/sync/conflict-resolution.service.ts
View file

@ -575,12 +575,12 @@ export class ConflictResolutionService {
contentConflicts: LwwContentConflict[],
): Promise<void> {
const MAX_NAMED = 3;
const titles = await Promise.all(
const labels = await Promise.all(
contentConflicts
.slice(0, MAX_NAMED)
.map((conflict) => this._getContentConflictTitle(conflict.entityId)),
.map((conflict) => this._buildContentConflictLabel(conflict)),
);
const named = titles.map((title) => `"${escapeHtml(title)}"`).join(', ');
const named = labels.join(', ');
const taskList = contentConflicts.length > MAX_NAMED ? `${named} …` : named;
this.bannerService.open({
@ -591,6 +591,42 @@ export class ConflictResolutionService {
});
}
/**
* Builds the display label for one conflicted task inside the banner's task
* list. Normally just the (escaped, quoted) current title. When the discarded
* edit changed the title, the current title is the *kept* value — useless for
* double-checking on its own — so we also name the discarded title: `"kept"
* (discarded: "dropped")`. Both values are escaped (rendered via `[innerHTML]`,
* see `_showContentConflictBanner`).
*/
private async _buildContentConflictLabel(
conflict: LwwContentConflict,
): Promise<string> {
const keptTitle = await this._getContentConflictTitle(conflict.entityId);
const kept = `"${escapeHtml(keptTitle)}"`;
const discardedTitle = conflict.discardedTitle?.trim();
// Skip the annotation when nothing meaningful to add: no title was
// discarded, or the discarded title equals the current one. The equality
// case covers two situations, both correctly silenced: (a) both devices set
// the same title; (b) a title edit lost to a concurrent *other-field* remote
// win — the winner didn't touch the title, so the current state still shows
// the (now-rejected) local title, which equals the discarded value. In both
// an annotation would read `"X" (discarded: "X")` — pure noise, no divergence
// to point at — so we render just the current title. (For the common
// title-vs-title case the current title IS the winning value and differs
// from the discarded one, so the annotation shows.)
if (!discardedTitle || discardedTitle === keptTitle.trim()) {
return kept;
}
const discarded = `"${escapeHtml(discardedTitle)}"`;
return (
this.translateService?.instant(T.F.SYNC.B.CONTENT_CONFLICT_TITLE_CHANGE, {
kept,
discarded,
}) ?? `${kept} (discarded: ${discarded})`
);
}
private async _getContentConflictTitle(entityId: string): Promise<string> {
const entity = await this.getCurrentEntityState('TASK' as EntityType, entityId);
const title = (entity as { title?: string } | undefined)?.title;

88
src/app/op-log/sync/lww-conflict-summary.util.spec.ts
View file

@ -70,7 +70,13 @@ describe('findLwwContentConflicts', () => {
payloadKeyFor,
);
expect(result).toEqual([{ entityId: 'task-1', discardedFields: ['title'] }]);
expect(result).toEqual([
{
entityId: 'task-1',
discardedFields: ['title'],
discardedTitle: 'My local title',
},
]);
});
it('classifies a discarded scheduling edit as routine (no content conflict)', () => {
@ -216,7 +222,9 @@ describe('findLwwContentConflicts', () => {
payloadKeyFor,
);
expect(result).toEqual([{ entityId: 'task-1', discardedFields: ['title'] }]);
expect(result).toEqual([
{ entityId: 'task-1', discardedFields: ['title'], discardedTitle: 'local' },
]);
});
it('merges distinct discarded content fields for the same task', () => {
@ -236,6 +244,80 @@ describe('findLwwContentConflicts', () => {
payloadKeyFor,
);
expect(result).toEqual([{ entityId: 'task-1', discardedFields: ['title', 'notes'] }]);
expect(result).toEqual([
{
entityId: 'task-1',
discardedFields: ['title', 'notes'],
discardedTitle: 'local',
},
]);
});
it('omits discardedTitle when the discarded edit did not touch the title', () => {
const result = findLwwContentConflicts(
[
resolution(
'remote',
[wrappedUpdate({ notes: 'lost note' })],
[wrappedUpdate({ dueDay: null })],
),
],
payloadKeyFor,
);
expect(result).toEqual([{ entityId: 'task-1', discardedFields: ['notes'] }]);
expect('discardedTitle' in result[0]).toBe(false);
});
it('ignores empty/whitespace discarded titles, keeping the real one', () => {
// A concurrent discarded title-clear must not blank out a genuine discarded
// rename in the same batch.
const result = findLwwContentConflicts(
[
resolution(
'remote',
[wrappedUpdate({ title: ' ' })],
[wrappedUpdate({ dueDay: null })],
),
resolution(
'remote',
[wrappedUpdate({ title: 'real discarded title' })],
[wrappedUpdate({ notes: 'x' })],
),
],
payloadKeyFor,
);
expect(result).toEqual([
{
entityId: 'task-1',
discardedFields: ['title'],
discardedTitle: 'real discarded title',
},
]);
});
it('surfaces the LAST discarded title (final rename), not the first', () => {
// User renamed the task twice offline (A -> B), both discarded on a remote
// win. The final value B is what they will look for.
const result = findLwwContentConflicts(
[
resolution(
'remote',
[wrappedUpdate({ title: 'rename A' })],
[wrappedUpdate({ dueDay: null })],
),
resolution(
'remote',
[wrappedUpdate({ title: 'rename B' })],
[wrappedUpdate({ notes: 'x' })],
),
],
payloadKeyFor,
);
expect(result).toEqual([
{ entityId: 'task-1', discardedFields: ['title'], discardedTitle: 'rename B' },
]);
});
});

40
src/app/op-log/sync/lww-conflict-summary.util.ts
View file

@ -25,6 +25,16 @@ export interface LwwContentConflict {
entityId: string;
/** The content fields the discarded edit(s) changed. */
discardedFields: string[];
/**
* The title value the discarded edit set, when the discarded edit changed the
* title. For a title conflict the current (kept) title is the *winning* value,
* so naming the task by it alone gives the user nothing to double-check — this
* is the value that was dropped, so the banner can show "kept X, discarded Y".
* Absent when no discarded edit touched the title (or it only cleared it to
* empty). The LAST non-empty discarded title in the batch wins — the user's
* final rename (deterministic given op order). Never logged (#9).
*/
discardedTitle?: string;
}
/**
@ -43,7 +53,7 @@ export const findLwwContentConflicts = (
resolutions: LwwResolvedConflict<Operation, EntityConflict>[],
payloadKeyFor: (entityType: string) => string,
): LwwContentConflict[] => {
const fieldsByTask = new Map<string, Set<string>>();
const byTask = new Map<string, { fields: Set<string>; discardedTitle?: string }>();
for (const { winner, conflict } of resolutions) {
if (conflict.entityType !== 'TASK') {
@ -58,18 +68,34 @@ export const findLwwContentConflicts = (
if (op.opType !== OpType.Update) {
continue;
}
for (const field of Object.keys(extractUpdateChanges(op.payload, payloadKey))) {
if (TASK_CONTENT_FIELDS.includes(field)) {
const fields = fieldsByTask.get(conflict.entityId) ?? new Set<string>();
fields.add(field);
fieldsByTask.set(conflict.entityId, fields);
const changes = extractUpdateChanges(op.payload, payloadKey);
for (const field of Object.keys(changes)) {
if (!TASK_CONTENT_FIELDS.includes(field)) {
continue;
}
const acc = byTask.get(conflict.entityId) ?? { fields: new Set<string>() };
acc.fields.add(field);
// Keep the LAST non-empty discarded title so the banner names the user's
// final rename, not a stale intermediate one (offline A→B→C, all
// discarded → show C). Ops are processed in append order, so a later
// non-empty value overwrites an earlier one. See
// LwwContentConflict.discardedTitle.
if (field === 'title') {
const value = (changes as { title?: unknown }).title;
if (typeof value === 'string' && value.trim().length) {
acc.discardedTitle = value;
}
}
byTask.set(conflict.entityId, acc);
}
}
}
return [...fieldsByTask].map(([entityId, fields]) => ({
return [...byTask].map(([entityId, { fields, discardedTitle }]) => ({
entityId,
discardedFields: [...fields],
// Omit the key entirely when no title was discarded so routine callers and
// tests keep the minimal { entityId, discardedFields } shape.
...(discardedTitle !== undefined ? { discardedTitle } : {}),
}));
};

1
src/app/t.const.ts
View file

@ -1227,6 +1227,7 @@ const T = {
},
B: {
CONTENT_CONFLICT_RESOLVED: 'F.SYNC.B.CONTENT_CONFLICT_RESOLVED',
CONTENT_CONFLICT_TITLE_CHANGE: 'F.SYNC.B.CONTENT_CONFLICT_TITLE_CHANGE',
CONTENT_CONFLICT_UNTITLED: 'F.SYNC.B.CONTENT_CONFLICT_UNTITLED',
},
BTN_RESTORE_FROM_HISTORY: 'F.SYNC.BTN_RESTORE_FROM_HISTORY',

1
src/assets/i18n/en.json
View file

@ -1204,6 +1204,7 @@
},
"B": {
"CONTENT_CONFLICT_RESOLVED": "Conflicting edits to {{taskList}} were auto-resolved by keeping the most recent version. Older edits may have been discarded — please double-check.",
"CONTENT_CONFLICT_TITLE_CHANGE": "{{kept}} (discarded: {{discarded}})",
"CONTENT_CONFLICT_UNTITLED": "Untitled task"
},
"BTN_RESTORE_FROM_HISTORY": "Restore from History",

2
src/assets/i18n/fr.json
View file

@ -2054,7 +2054,7 @@
"CONFIRM": "Confirmer",
"CONTINUE": "Continuer",
"DELETE": "Effacer",
"DISMISS": "Rejeter",
"DISMISS": "Ignorer",
"DO_IT": "Fais le !",
"DUPLICATE": "Dupliquer",
"DURATION_DESCRIPTION": "ex: \"5h 23m\" ce qui équivaut à 5 heures et 23 minutes",