Skip to content

Commit 58d67c9

Browse files
committed
fix: ask about migration at most once and never block after that
The dialog could reappear on every boot. Declining wrote nothing, and the done flag was only written on success, so saying no or losing the connection part way meant being asked again the next time the app opened, indefinitely. A prompted flag is now recorded before the question is even answered, so declining, closing the tab on the dialog, or a transfer that broke half way all count as having been asked. The automatic path never raises it again; the Help menu entry remains for a deliberate retry. An interrupted run used to fail silently on the automatic path, because the error was only reported when the user had started it from the menu. Someone who opted in and watched a task appear would then see nothing at all. Failures after the user accepts are now always reported, whichever path started it, and say that what already copied has been kept. Everything after the single question is now a notification rather than another modal: completion, partial failure, interruption, and the nothing-to-migrate and cannot-reach outcomes of a manual run. The completion notification carries the reload action, since copied extensions only load on the next boot. Exactly one modal dialog remains in the whole feature, the initial ask.
1 parent 004a9be commit 58d67c9

5 files changed

Lines changed: 97 additions & 34 deletions

File tree

docs/API-Reference/command/Commands.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -854,6 +854,12 @@ Checks for updates
854854
## HELP\_AUTO\_UPDATE
855855
Toggles auto update
856856

857+
**Kind**: global variable
858+
<a name="HELP_MIGRATE_DATA"></a>
859+
860+
## HELP\_MIGRATE\_DATA
861+
Migrates browser data from the legacy web origin
862+
857863
**Kind**: global variable
858864
<a name="CMD_WORKINGSET_SORT_BY_ADDED"></a>
859865

src/extensionsIntegrated/MigrateAssist/constants.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,15 @@ define(function (require, exports, module) {
7272
*/
7373
const MIGRATION_DONE_KEY = "migrateAssist.v1.done";
7474

75+
/**
76+
* PhStore key recording that the user has already been asked. The automatic path prompts at most
77+
* once, whatever the outcome: declining, closing the tab, or a transfer that broke half way all
78+
* count as having been asked. Re-prompting on every boot after any of those would be a nag, and
79+
* the Help menu entry is always there for a deliberate retry.
80+
* @type {string}
81+
*/
82+
const MIGRATION_PROMPTED_KEY = "migrateAssist.v1.prompted";
83+
7584
/**
7685
* Dev only override, so the whole cross origin flow can be exercised on one dev server.
7786
* http://localhost:8000 and http://127.0.0.1:8000 are different origins with separate IndexedDB
@@ -203,6 +212,7 @@ define(function (require, exports, module) {
203212
exports.SUNSET_DATE = SUNSET_DATE;
204213
exports.TWA_STORE_URL = TWA_STORE_URL;
205214
exports.MIGRATION_DONE_KEY = MIGRATION_DONE_KEY;
215+
exports.MIGRATION_PROMPTED_KEY = MIGRATION_PROMPTED_KEY;
206216
exports.getLegacyOrigin = getLegacyOrigin;
207217
exports.getMigrateAssistURL = getMigrateAssistURL;
208218
exports.getLegacyDomainName = getLegacyDomainName;

src/extensionsIntegrated/MigrateAssist/migrator.js

Lines changed: 64 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
define(function (require, exports, module) {
3636
const Dialogs = require("widgets/Dialogs"),
3737
DefaultDialogs = require("widgets/DefaultDialogs"),
38+
NotificationUI = require("widgets/NotificationUI"),
3839
Strings = require("strings"),
3940
StringUtils = require("utils/StringUtils"),
4041
Metrics = require("utils/Metrics"),
@@ -51,6 +52,7 @@ define(function (require, exports, module) {
5152

5253
const RESULT_MIGRATED = "migrated",
5354
RESULT_DECLINED = "declined",
55+
RESULT_INTERRUPTED = "interrupted",
5456
RESULT_NOTHING = "nothing",
5557
RESULT_UNREACHABLE = "unreachable";
5658

@@ -235,32 +237,39 @@ define(function (require, exports, module) {
235237
).getPromise();
236238
}
237239

240+
/**
241+
* Everything after the single up front question is reported at the bottom of the window rather
242+
* than in another modal. The user opted in and went back to work; interrupting them again to say
243+
* it finished would undo the point of moving progress out of a dialog in the first place.
244+
*/
245+
function _toast(title, message, style, $extra) {
246+
const $content = $("<div>").append($("<div>").text(message));
247+
if ($extra) {
248+
$content.append($extra);
249+
}
250+
return NotificationUI.createToastFromTemplate(title, $content, {
251+
dismissOnClick: false, // there is a close button, and a stray click must not eat the action
252+
toastStyle: style
253+
});
254+
}
255+
238256
function _showCompletion(migratedFiles, failed) {
239-
const message = failed.length
240-
? StringUtils.format(Strings.MIGRATE_DONE_MESSAGE, migratedFiles) + "<br><br>"
241-
+ StringUtils.format(Strings.MIGRATE_DONE_PARTIAL, failed.length)
242-
: StringUtils.format(Strings.MIGRATE_DONE_MESSAGE, migratedFiles);
243-
Dialogs.showModalDialog(
244-
DefaultDialogs.DIALOG_ID_INFO,
245-
Strings.MIGRATE_DONE_TITLE,
246-
message,
247-
[
248-
{
249-
className: Dialogs.DIALOG_BTN_CLASS_NORMAL,
250-
id: Dialogs.DIALOG_BTN_CANCEL,
251-
text: Strings.MIGRATE_RELOAD_LATER
252-
},
253-
{
254-
className: Dialogs.DIALOG_BTN_CLASS_PRIMARY,
255-
id: Dialogs.DIALOG_BTN_OK,
256-
text: Strings.MIGRATE_RELOAD_NOW
257-
}
258-
]
259-
).done(function (buttonId) {
260-
if (buttonId === Dialogs.DIALOG_BTN_OK) {
261-
CommandManager.execute(Commands.APP_RELOAD);
262-
}
257+
const $actions = $("<div>").addClass("migrate-assist-toast-actions");
258+
const $reload = $("<button>").addClass("btn primary btn-mini")
259+
.text(Strings.MIGRATE_RELOAD_NOW);
260+
$reload.on("click", function () {
261+
CommandManager.execute(Commands.APP_RELOAD);
263262
});
263+
$actions.append($reload);
264+
if (failed.length) {
265+
$actions.prepend($("<div>").addClass("migrate-assist-toast-detail")
266+
.text(StringUtils.format(Strings.MIGRATE_DONE_PARTIAL, failed.length)));
267+
}
268+
_toast(Strings.MIGRATE_DONE_TITLE,
269+
StringUtils.format(Strings.MIGRATE_DONE_MESSAGE, migratedFiles),
270+
failed.length ? NotificationUI.NOTIFICATION_STYLES_CSS_CLASS.WARNING
271+
: NotificationUI.NOTIFICATION_STYLES_CSS_CLASS.SUCCESS,
272+
$actions);
264273
}
265274

266275
/**
@@ -306,6 +315,7 @@ define(function (require, exports, module) {
306315
migrationRunning = true;
307316
const bridge = _createBridge();
308317
let progress = null;
318+
let userAccepted = false;
309319
try {
310320
const scan = await bridge.scan();
311321
if (!scan.hasData || !scan.files.length) {
@@ -314,12 +324,17 @@ define(function (require, exports, module) {
314324
return RESULT_NOTHING;
315325
}
316326

327+
// Recorded before the dialog is even answered, so closing the tab on it counts as
328+
// having been asked. The automatic path will not raise it again.
329+
PhStore.setItem(Constants.MIGRATION_PROMPTED_KEY, { at: Date.now() });
330+
317331
// Asked once, before anything is copied. After this the user is left alone.
318332
const choice = await _confirmStart(scan.files.length);
319333
if (choice !== Dialogs.DIALOG_BTN_OK) {
320334
Metrics.countEvent(Metrics.EVENT_TYPE.PLATFORM, "migrateAssist", "declined");
321335
return RESULT_DECLINED;
322336
}
337+
userAccepted = true;
323338

324339
Metrics.countEvent(Metrics.EVENT_TYPE.PLATFORM, "migrateAssist",
325340
manual ? "manualStart" : "autoStart");
@@ -366,11 +381,21 @@ define(function (require, exports, module) {
366381
return RESULT_MIGRATED;
367382
} catch (err) {
368383
console.error("MigrateAssist: migration could not run", err);
369-
Metrics.countEvent(Metrics.EVENT_TYPE.PLATFORM, "migrateAssist",
370-
manual ? "manualUnreachable" : "autoUnreachable");
371384
if (progress) {
372385
progress.fail();
373386
}
387+
if (userAccepted) {
388+
// They opted in and watched a task start, so a silent stop is not acceptable even on
389+
// the automatic path. Whatever landed before the break stays; a retry overwrites it.
390+
Metrics.countEvent(Metrics.EVENT_TYPE.PLATFORM, "migrateAssist", "interrupted");
391+
_toast(Strings.MIGRATE_INTERRUPTED_TITLE,
392+
StringUtils.format(Strings.MIGRATE_INTERRUPTED_MESSAGE,
393+
Constants.getLegacyDomainName()),
394+
NotificationUI.NOTIFICATION_STYLES_CSS_CLASS.ERROR);
395+
return RESULT_INTERRUPTED;
396+
}
397+
Metrics.countEvent(Metrics.EVENT_TYPE.PLATFORM, "migrateAssist",
398+
manual ? "manualUnreachable" : "autoUnreachable");
374399
return RESULT_UNREACHABLE;
375400
} finally {
376401
migrationRunning = false;
@@ -392,6 +417,10 @@ define(function (require, exports, module) {
392417
if (PhStore.getItem(Constants.MIGRATION_DONE_KEY)) {
393418
return;
394419
}
420+
// Already asked once. Anything further is on the user, from the Help menu.
421+
if (PhStore.getItem(Constants.MIGRATION_PROMPTED_KEY)) {
422+
return;
423+
}
395424
// Once the legacy origin is gone there is nothing to probe, so the feature disables itself
396425
// rather than opening a doomed iframe on every boot forever.
397426
if (Constants.isPastSunset()) {
@@ -406,17 +435,18 @@ define(function (require, exports, module) {
406435
*/
407436
async function runManually() {
408437
const result = await run(true);
409-
if (result === RESULT_DECLINED) {
410-
return; // the user said no, they do not need to be told what they just chose
438+
if (result === RESULT_DECLINED || result === RESULT_INTERRUPTED) {
439+
// Declining needs no confirmation, and an interrupted run has already said so itself.
440+
return;
411441
}
412442
if (result === RESULT_NOTHING) {
413-
Dialogs.showModalDialog(DefaultDialogs.DIALOG_ID_INFO,
414-
Strings.MIGRATE_NOTHING_TITLE,
415-
StringUtils.format(Strings.MIGRATE_NOTHING_MESSAGE, Constants.getLegacyDomainName()));
443+
_toast(Strings.MIGRATE_NOTHING_TITLE,
444+
StringUtils.format(Strings.MIGRATE_NOTHING_MESSAGE, Constants.getLegacyDomainName()),
445+
NotificationUI.NOTIFICATION_STYLES_CSS_CLASS.INFO);
416446
} else if (result === RESULT_UNREACHABLE) {
417-
Dialogs.showModalDialog(DefaultDialogs.DIALOG_ID_ERROR,
418-
Strings.MIGRATE_UNREACHABLE_TITLE,
419-
StringUtils.format(Strings.MIGRATE_UNREACHABLE_MESSAGE, Constants.getLegacyDomainName()));
447+
_toast(Strings.MIGRATE_UNREACHABLE_TITLE,
448+
StringUtils.format(Strings.MIGRATE_UNREACHABLE_MESSAGE, Constants.getLegacyDomainName()),
449+
NotificationUI.NOTIFICATION_STYLES_CSS_CLASS.ERROR);
420450
}
421451
}
422452

src/nls/root/strings.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2942,6 +2942,8 @@ define({
29422942
"MIGRATE_RELOAD_NOW": "Reload",
29432943
"MIGRATE_NOTHING_TITLE": "Nothing to bring over",
29442944
"MIGRATE_NOTHING_MESSAGE": "We could not find any projects, settings or extensions on {0} that need copying.",
2945+
"MIGRATE_INTERRUPTED_TITLE": "Migration did not finish",
2946+
"MIGRATE_INTERRUPTED_MESSAGE": "The connection to {0} was lost part way through. Anything already copied has been kept. You can pick up where this left off any time from the Help menu.",
29452947
"MIGRATE_UNREACHABLE_TITLE": "Could not reach the old site",
29462948
"MIGRATE_UNREACHABLE_MESSAGE": "{APP_NAME} could not connect to {0} to check for your data. Please check your connection and try again.",
29472949
"MIGRATE_START_MESSAGE": "Found {0} files on {1} to bring over. This runs in the background and you can keep working while it happens. It only needs to be done once.",

src/styles/Extn-MigrateAssist.less

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,3 +64,18 @@
6464
background-position: -200% 0;
6565
}
6666
}
67+
68+
// Completion toast: the reload action and the partial-failure note sit under the message.
69+
.migrate-assist-toast-actions {
70+
margin-top: 10px;
71+
display: flex;
72+
align-items: center;
73+
justify-content: flex-end;
74+
gap: 8px;
75+
}
76+
77+
.migrate-assist-toast-detail {
78+
margin-right: auto;
79+
font-size: 0.9em;
80+
opacity: 0.8;
81+
}

0 commit comments

Comments
 (0)