Mobile: Resolves #10245: Allow marking items as "ignored" in sync status (#10261)

.eslintignore

@@ -597,6 +597,7 @@ packages/app-mobile/components/screens/Notes.js
 packages/app-mobile/components/screens/UpgradeSyncTargetScreen.js
 packages/app-mobile/components/screens/encryption-config.js
 packages/app-mobile/components/screens/search.js
+packages/app-mobile/components/screens/status.js
 packages/app-mobile/components/side-menu-content.js
 packages/app-mobile/components/voiceTyping/VoiceTypingDialog.js
 packages/app-mobile/gulpfile.js
@@ -858,6 +859,7 @@ packages/lib/models/Tag.test.js
 packages/lib/models/Tag.js
 packages/lib/models/dateTimeFormats.test.js
 packages/lib/models/settings/FileHandler.js
+packages/lib/models/settings/settingValidations.test.js
 packages/lib/models/settings/settingValidations.js
 packages/lib/models/utils/getCollator.js
 packages/lib/models/utils/getConflictFolderId.js
@@ -897,6 +899,7 @@ packages/lib/services/KvStore.js
 packages/lib/services/MigrationService.js
 packages/lib/services/NavService.js
 packages/lib/services/PostMessageService.js
+packages/lib/services/ReportService.test.js
 packages/lib/services/ReportService.js
 packages/lib/services/ResourceEditWatcher/index.js
 packages/lib/services/ResourceEditWatcher/reducer.js
@@ -924,6 +927,7 @@ packages/lib/services/database/migrations/43.js
 packages/lib/services/database/migrations/44.js
 packages/lib/services/database/migrations/45.js
 packages/lib/services/database/migrations/46.js
+packages/lib/services/database/migrations/47.js
 packages/lib/services/database/migrations/index.js
 packages/lib/services/database/sqlStringToLines.js
 packages/lib/services/database/types.js

.gitignore

@@ -577,6 +577,7 @@ packages/app-mobile/components/screens/Notes.js
 packages/app-mobile/components/screens/UpgradeSyncTargetScreen.js
 packages/app-mobile/components/screens/encryption-config.js
 packages/app-mobile/components/screens/search.js
+packages/app-mobile/components/screens/status.js
 packages/app-mobile/components/side-menu-content.js
 packages/app-mobile/components/voiceTyping/VoiceTypingDialog.js
 packages/app-mobile/gulpfile.js
@@ -838,6 +839,7 @@ packages/lib/models/Tag.test.js
 packages/lib/models/Tag.js
 packages/lib/models/dateTimeFormats.test.js
 packages/lib/models/settings/FileHandler.js
+packages/lib/models/settings/settingValidations.test.js
 packages/lib/models/settings/settingValidations.js
 packages/lib/models/utils/getCollator.js
 packages/lib/models/utils/getConflictFolderId.js
@@ -877,6 +879,7 @@ packages/lib/services/KvStore.js
 packages/lib/services/MigrationService.js
 packages/lib/services/NavService.js
 packages/lib/services/PostMessageService.js
+packages/lib/services/ReportService.test.js
 packages/lib/services/ReportService.js
 packages/lib/services/ResourceEditWatcher/index.js
 packages/lib/services/ResourceEditWatcher/reducer.js
@@ -904,6 +907,7 @@ packages/lib/services/database/migrations/43.js
 packages/lib/services/database/migrations/44.js
 packages/lib/services/database/migrations/45.js
 packages/lib/services/database/migrations/46.js
+packages/lib/services/database/migrations/47.js
 packages/lib/services/database/migrations/index.js
 packages/lib/services/database/sqlStringToLines.js
 packages/lib/services/database/types.js

packages/app-mobile/components/screens/status.tsx

@@ -1,50 +1,64 @@
-const React = require('react');
+import * as React from 'react';
 
-const { View, Text, Button, FlatList } = require('react-native');
+import { View, Text, Button, FlatList, TextStyle, StyleSheet } from 'react-native';
-const Setting = require('@joplin/lib/models/Setting').default;
+import Setting from '@joplin/lib/models/Setting';
-const { connect } = require('react-redux');
+import { connect } from 'react-redux';
-const { ScreenHeader } = require('../ScreenHeader');
+import { ScreenHeader } from '../ScreenHeader';
-const ReportService = require('@joplin/lib/services/ReportService').default;
+import ReportService, { ReportSection } from '@joplin/lib/services/ReportService';
-const { _ } = require('@joplin/lib/locale');
+import { _ } from '@joplin/lib/locale';
-const { BaseScreenComponent } = require('../base-screen');
+import { BaseScreenComponent } from '../base-screen';
-const { themeStyle } = require('../global-style');
+import { themeStyle } from '../global-style';
+import { AppState } from '../../utils/types';
+import checkDisabledSyncItemsNotification from '@joplin/lib/services/synchronizer/utils/checkDisabledSyncItemsNotification';
+import { Dispatch } from 'redux';
 
-class StatusScreenComponent extends BaseScreenComponent {
+interface Props {
-	static navigationOptions() {
+	themeId: number;
-		return { header: null };
+	dispatch: Dispatch;
-	}
+}
 
-	constructor() {
+interface State {
-		super();
+	report: ReportSection[];
+}
+
+class StatusScreenComponent extends BaseScreenComponent<Props, State> {
+	public constructor(props: Props) {
+		super(props);
 		this.state = {
 			report: [],
 		};
 	}
 
-	UNSAFE_componentWillMount() {
+	public override componentDidMount() {
-		this.resfreshScreen();
+		void this.refreshScreen();
 	}
 
-	async resfreshScreen() {
+	private async refreshScreen() {
 		const service = new ReportService();
 		const report = await service.status(Setting.value('sync.target'));
 		this.setState({ report: report });
 	}
 
-	styles() {
+	private styles() {
 		const theme = themeStyle(this.props.themeId);
-		return {
+		return StyleSheet.create({
 			body: {
 				flex: 1,
 				margin: theme.margin,
 			},
-		};
+			actionButton: {
+				flex: 0,
+				marginLeft: 2,
+				marginRight: 2,
+			},
+		});
 	}
 
-	render() {
+	public override render() {
 		const theme = themeStyle(this.props.themeId);
+		const styles = this.styles();
 
-		const renderBody = report => {
+		const renderBody = (report: ReportSection[]) => {
 			const baseStyle = {
 				paddingLeft: 6,
 				paddingRight: 6,
@@ -60,7 +74,7 @@ class StatusScreenComponent extends BaseScreenComponent {
 			for (let i = 0; i < report.length; i++) {
 				const section = report[i];
 
-				let style = { ...baseStyle };
+				let style: TextStyle = { ...baseStyle };
 				style.fontWeight = 'bold';
 				if (i > 0) style.paddingTop = 20;
 				lines.push({ key: `section_${i}`, isSection: true, text: section.title });
@@ -76,11 +90,19 @@ class StatusScreenComponent extends BaseScreenComponent {
 					let text = '';
 
 					let retryHandler = null;
+					let ignoreHandler = null;
 					if (typeof item === 'object') {
 						if (item.canRetry) {
 							retryHandler = async () => {
 								await item.retryHandler();
-								this.resfreshScreen();
+								await this.refreshScreen();
+							};
+						}
+						if (item.canIgnore) {
+							ignoreHandler = async () => {
+								await item.ignoreHandler();
+								await this.refreshScreen();
+								await checkDisabledSyncItemsNotification((action) => this.props.dispatch(action));
 							};
 						}
 						text = item.text;
@@ -88,7 +110,7 @@ class StatusScreenComponent extends BaseScreenComponent {
 						text = item;
 					}
 
-					lines.push({ key: `item_${i}_${n}`, text: text, retryHandler: retryHandler });
+					lines.push({ key: `item_${i}_${n}`, text: text, retryHandler, ignoreHandler });
 				}
 
 				lines.push({ key: `divider2_${i}`, isDivider: true });
@@ -98,7 +120,7 @@ class StatusScreenComponent extends BaseScreenComponent {
 				<FlatList
 					data={lines}
 					renderItem={({ item }) => {
-						const style = { ...baseStyle };
+						const style: TextStyle = { ...baseStyle };
 
 						if (item.isSection === true) {
 							style.fontWeight = 'bold';
@@ -114,17 +136,24 @@ class StatusScreenComponent extends BaseScreenComponent {
 						) : null;
 
 						const retryButton = item.retryHandler ? (
-							<View style={{ flex: 0 }}>
+							<View style={styles.actionButton}>
 								<Button title={_('Retry')} onPress={item.retryHandler} />
 							</View>
 						) : null;
 
+						const ignoreButton = item.ignoreHandler ? (
+							<View style={styles.actionButton}>
+								<Button title={_('Ignore')} onPress={item.ignoreHandler} />
+							</View>
+						) : null;
+
 						if (item.isDivider) {
 							return <View style={{ borderBottomWidth: 1, borderBottomColor: theme.dividerColor, marginTop: 20, marginBottom: 20 }} />;
 						} else {
 							return (
 								<View style={{ flex: 1, flexDirection: 'row' }}>
 									<Text style={style}>{item.text}</Text>
+									{ignoreButton}
 									{retryAllButton}
 									{retryButton}
 								</View>
@@ -140,17 +169,15 @@ class StatusScreenComponent extends BaseScreenComponent {
 		return (
 			<View style={this.rootStyle(this.props.themeId).root}>
 				<ScreenHeader title={_('Status')} />
-				<View style={this.styles().body}>{body}</View>
+				<View style={styles.body}>{body}</View>
-				<Button title={_('Refresh')} onPress={() => this.resfreshScreen()} />
+				<Button title={_('Refresh')} onPress={() => this.refreshScreen()} />
 			</View>
 		);
 	}
 }
 
-const StatusScreen = connect(state => {
+export default connect((state: AppState) => {
 	return {
 		themeId: state.settings.theme,
 	};
 })(StatusScreenComponent);
-
-module.exports = { StatusScreen };

packages/app-mobile/root.tsx

@@ -60,7 +60,7 @@ const { TagsScreen } = require('./components/screens/tags.js');
 import ConfigScreen from './components/screens/ConfigScreen/ConfigScreen';
 const { FolderScreen } = require('./components/screens/folder.js');
 import LogScreen from './components/screens/LogScreen';
-const { StatusScreen } = require('./components/screens/status.js');
+import StatusScreen from './components/screens/status';
 const { SearchScreen } = require('./components/screens/search.js');
 const { OneDriveLoginScreen } = require('./components/screens/onedrive-login.js');
 import EncryptionConfigScreen from './components/screens/encryption-config';

packages/lib/models/BaseItem.ts

@@ -14,6 +14,7 @@ import JoplinError from '../JoplinError';
 import { LoadOptions, SaveOptions } from './utils/types';
 import { State as ShareState } from '../services/share/reducer';
 import { checkIfItemCanBeAddedToFolder, checkIfItemCanBeChanged, checkIfItemsCanBeChanged, needsShareReadOnlyChecks } from './utils/readOnly';
+import { checkObjectHasProperties } from '@joplin/utils/object';
 
 const { sprintf } = require('sprintf-js');
 const moment = require('moment');
@@ -820,16 +821,26 @@ export default class BaseItem extends BaseModel {
 				syncInfo: row,
 				location: row.item_location,
 				item: item,
+				warning_ignored: row.sync_warning_ignored,
 			});
 		}
 		return output;
 	}
 
-	public static async syncDisabledItemsCount(syncTargetId: number) {
+	public static async syncDisabledItemsCount(syncTargetId: number, includeIgnored = false) {
-		const r = await this.db().selectOne('SELECT count(*) as total FROM sync_items WHERE sync_disabled = 1 AND sync_target = ?', [syncTargetId]);
+		const whereQueries = ['sync_disabled = 1', 'sync_target = ?'];
+		const whereArgs = [syncTargetId];
+		if (!includeIgnored) {
+			whereQueries.push('sync_warning_ignored = 0');
+		}
+		const r = await this.db().selectOne(`SELECT count(*) as total FROM sync_items WHERE ${whereQueries.join(' AND ')}`, whereArgs);
 		return r ? r.total : 0;
 	}
 
+	public static async syncDisabledItemsCountIncludingIgnored(syncTargetId: number) {
+		return this.syncDisabledItemsCount(syncTargetId, true);
+	}
+
 	// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
 	public static updateSyncTimeQueries(syncTarget: number, item: any, syncTime: number, syncDisabled = false, syncDisabledReason = '', itemLocation: number = null) {
 		const itemType = item.type_;
@@ -867,6 +878,15 @@ export default class BaseItem extends BaseModel {
 		await this.db().exec('DELETE FROM sync_items WHERE item_type = ? AND item_id = ?', [itemType, itemId]);
 	}
 
+	public static async ignoreItemSyncWarning(syncTarget: number, item: { type_?: number; id?: string }) {
+		checkObjectHasProperties(item, ['type_', 'id']);
+		const itemType = item.type_;
+		const itemId = item.id;
+		const sql = 'UPDATE sync_items SET sync_warning_ignored = ? WHERE item_id = ? AND item_type = ? AND sync_target = ?';
+		const params = [1, itemId, itemType, syncTarget];
+		await this.db().exec(sql, params);
+	}
+
 	// When an item is deleted, its associated sync_items data is not immediately deleted for
 	// performance reason. So this function is used to look for these remaining sync_items and
 	// delete them.

packages/lib/models/settings/settingValidations.test.ts

@@ -0,0 +1,35 @@
+import SyncTargetRegistry from '../../SyncTargetRegistry';
+import { createNTestNotes, setupDatabaseAndSynchronizer, switchClient } from '../../testing/test-utils';
+import BaseItem from '../BaseItem';
+import Folder from '../Folder';
+import Setting from '../Setting';
+import settingValidations from './settingValidations';
+
+describe('settingValidations', () => {
+	beforeEach(async () => {
+		await setupDatabaseAndSynchronizer(1);
+		await switchClient(1);
+	});
+
+	test('sync disabled items should prevent switching sync targets unless ignored', async () => {
+		const folder = await Folder.save({ title: 'Test' });
+		const noteCount = 5;
+		const testNotes = await createNTestNotes(noteCount, folder);
+		const syncTargetId = SyncTargetRegistry.nameToId('memory');
+		Setting.setValue('sync.target', syncTargetId);
+
+		for (const testNote of testNotes) {
+			await BaseItem.saveSyncDisabled(syncTargetId, testNote, 'Disabled reason');
+		}
+
+		const newSyncTargetId = SyncTargetRegistry.nameToId('dropbox');
+		// Validation should fail with some error message.
+		expect(await settingValidations(['sync.target'], { 'sync.target': newSyncTargetId })).not.toBe('');
+
+		// Should pass after dismissing all warnings
+		for (const testNote of testNotes) {
+			await BaseItem.ignoreItemSyncWarning(syncTargetId, testNote);
+		}
+		expect(await settingValidations(['sync.target'], { 'sync.target': newSyncTargetId })).toBe('');
+	});
+});

packages/lib/services/ReportService.test.ts

@@ -0,0 +1,108 @@
+import SyncTargetRegistry from '../SyncTargetRegistry';
+import { _ } from '../locale';
+import ReportService, { ReportSection } from './ReportService';
+import { createNTestNotes, decryptionWorker, setupDatabaseAndSynchronizer, switchClient, synchronizerStart } from '../testing/test-utils';
+import Folder from '../models/Folder';
+import BaseItem from '../models/BaseItem';
+import DecryptionWorker from './DecryptionWorker';
+
+
+const getSectionsWithTitle = (report: ReportSection[], title: string) => {
+	return report.filter(section => section.title === title);
+};
+
+const getCannotSyncSection = (report: ReportSection[]) => {
+	return getSectionsWithTitle(report, _('Items that cannot be synchronised'))[0];
+};
+
+const getIgnoredSection = (report: ReportSection[]) => {
+	return getSectionsWithTitle(report, _('Ignored items that cannot be synchronised'))[0];
+};
+
+const sectionBodyToText = (section: ReportSection) => {
+	return section.body.map(item => {
+		if (typeof item === 'string') {
+			return item;
+		}
+
+		return item.text;
+	}).join('\n');
+};
+
+describe('ReportService', () => {
+	beforeEach(async () => {
+		await setupDatabaseAndSynchronizer(1);
+		await switchClient(1);
+		await synchronizerStart();
+		// For compatibility with code that calls DecryptionWorker.instance()
+		DecryptionWorker.instance_ = decryptionWorker();
+	});
+
+	it('should move sync errors to the "ignored" section after clicking "ignore"', async () => {
+		const folder = await Folder.save({ title: 'Test' });
+		const noteCount = 5;
+		const testNotes = await createNTestNotes(noteCount, folder);
+		await synchronizerStart();
+		const syncTargetId = SyncTargetRegistry.nameToId('memory');
+
+		const disabledReason = 'Test reason';
+		for (const testNote of testNotes) {
+			await BaseItem.saveSyncDisabled(syncTargetId, testNote, disabledReason);
+		}
+
+		const service = new ReportService();
+		let report = await service.status(syncTargetId);
+
+		// Items should all initially be listed as "cannot be synchronized", but should be ignorable.
+		const unsyncableSection = getCannotSyncSection(report);
+		const ignorableItems = [];
+		for (const item of unsyncableSection.body) {
+			if (typeof item === 'object' && item.canIgnore) {
+				ignorableItems.push(item);
+			}
+		}
+		expect(ignorableItems).toHaveLength(noteCount);
+		expect(sectionBodyToText(unsyncableSection)).toContain(disabledReason);
+
+		// Ignore all
+		expect(await BaseItem.syncDisabledItemsCount(syncTargetId)).toBe(noteCount);
+		expect(await BaseItem.syncDisabledItemsCountIncludingIgnored(syncTargetId)).toBe(noteCount);
+		for (const item of ignorableItems) {
+			await item.ignoreHandler();
+		}
+		expect(await BaseItem.syncDisabledItemsCount(syncTargetId)).toBe(0);
+		expect(await BaseItem.syncDisabledItemsCountIncludingIgnored(syncTargetId)).toBe(noteCount);
+
+		await synchronizerStart();
+		report = await service.status(syncTargetId);
+
+		// Should now be in the ignored section
+		const ignoredSection = getIgnoredSection(report);
+		expect(ignoredSection).toBeTruthy();
+		expect(sectionBodyToText(unsyncableSection)).toContain(disabledReason);
+		expect(sectionBodyToText(getCannotSyncSection(report))).not.toContain(disabledReason);
+
+		// Should not be possible to re-ignore an item in the ignored section
+		let ignoredItemCount = 0;
+		for (const item of ignoredSection.body) {
+			if (typeof item === 'object' && item.text?.includes(disabledReason)) {
+				expect(item.canIgnore).toBeFalsy();
+				expect(item.canRetry).toBe(true);
+				ignoredItemCount++;
+			}
+		}
+		// Should have the correct number of ignored items
+		expect(await BaseItem.syncDisabledItemsCountIncludingIgnored(syncTargetId)).toBe(ignoredItemCount);
+		expect(ignoredItemCount).toBe(noteCount);
+
+		// Clicking "retry" should un-ignore
+		for (const item of ignoredSection.body) {
+			if (typeof item === 'object' && item.text?.includes(disabledReason)) {
+				expect(item.canRetry).toBe(true);
+				await item.retryHandler();
+				break;
+			}
+		}
+		expect(await BaseItem.syncDisabledItemsCountIncludingIgnored(syncTargetId)).toBe(noteCount - 1);
+	});
+});

packages/lib/services/ReportService.ts

@@ -24,6 +24,8 @@ enum ReportItemType {
 type ReportItemOrString = ReportItem | string;
 
 export type RetryAllHandler = ()=> void;
+export type RetryHandler = ()=> Promise<void>;
+export type IgnoreHandler = ()=> Promise<void>;
 
 export interface ReportSection {
 	title: string;
@@ -39,7 +41,9 @@ export interface ReportItem {
 	text?: string;
 	canRetry?: boolean;
 	canRetryType?: CanRetryType;
-	retryHandler?: RetryAllHandler;
+	retryHandler?: RetryHandler;
+	canIgnore?: boolean;
+	ignoreHandler?: IgnoreHandler;
 }
 
 export default class ReportService {
@@ -182,10 +186,7 @@ export default class ReportService {
 
 			section.body.push(_('These items will remain on the device but will not be uploaded to the sync target. In order to find these items, either search for the title or the ID (which is displayed in brackets above).'));
 
-			section.body.push({ type: ReportItemType.OpenList, key: 'disabledSyncItems' });
+			const processRow = (row: typeof disabledItems[0]) => {
-
-			for (let i = 0; i < disabledItems.length; i++) {
-				const row = disabledItems[i];
 				let msg = '';
 				if (row.location === BaseItem.SYNC_ITEM_LOCATION_LOCAL) {
 					msg = _('%s (%s) could not be uploaded: %s', row.item.title, row.item.id, row.syncInfo.sync_disabled_reason);
@@ -200,14 +201,47 @@ export default class ReportService {
 					retryHandler: async () => {
 						await BaseItem.saveSyncEnabled(row.item.type_, row.item.id);
 					},
+					canIgnore: !row.warning_ignored,
+					ignoreHandler: async () => {
+						await BaseItem.ignoreItemSyncWarning(syncTarget, row.item);
+					},
 				});
+			};
+
+			section.body.push({ type: ReportItemType.OpenList, key: 'disabledSyncItems' });
+
+			let hasIgnoredItems = false;
+			let hasUnignoredItems = false;
+			for (const row of disabledItems) {
+				if (!row.warning_ignored) {
+					processRow(row);
+					hasUnignoredItems = true;
+				} else {
+					hasIgnoredItems = true;
+				}
+			}
+
+			if (!hasUnignoredItems) {
+				section.body.push(_('All item sync failures have been marked as "ignored".'));
 			}
 
 			section.body.push({ type: ReportItemType.CloseList });
-
 			section = this.addRetryAllHandler(section);
-
 			sections.push(section);
+
+
+			if (hasIgnoredItems) {
+				section = { title: _('Ignored items that cannot be synchronised'), body: [] };
+				section.body.push(_('These items failed to sync, but have been marked as "ignored". They won\'t cause the sync warning to appear, but still aren\'t synced. To unignore, click "retry".'));
+				section.body.push({ type: ReportItemType.OpenList, key: 'ignoredDisabledItems' });
+				for (const row of disabledItems) {
+					if (row.warning_ignored) {
+						processRow(row);
+					}
+				}
+				section.body.push({ type: ReportItemType.CloseList });
+				sections.push(section);
+			}
 		}
 
 		const decryptionDisabledItems = await DecryptionWorker.instance().decryptionDisabledItems();

packages/lib/services/database/migrations/47.ts

@@ -0,0 +1,7 @@
+import { SqlQuery } from '../types';
+
+export default (): (SqlQuery|string)[] => {
+	return [
+		'ALTER TABLE sync_items ADD COLUMN sync_warning_ignored INT NOT NULL DEFAULT "0"',
+	];
+};

packages/lib/services/database/migrations/index.ts

@@ -4,6 +4,7 @@ import migration43 from './43';
 import migration44 from './44';
 import migration45 from './45';
 import migration46 from './46';
+import migration47 from './47';
 
 import { Migration } from '../types';
 
@@ -13,6 +14,7 @@ const index: Migration[] = [
 	migration44,
 	migration45,
 	migration46,
+	migration47,
 ];
 
 export default index;

packages/lib/services/database/types.ts

@@ -330,6 +330,7 @@ export interface SyncItemEntity {
   'item_type'?: number;
   'sync_disabled'?: number;
   'sync_disabled_reason'?: string;
+  'sync_warning_ignored'?: number;
   'sync_target'?: number;
   'sync_time'?: number;
   'type_'?: number;
@@ -434,6 +435,7 @@ export const databaseSchema: DatabaseTables = {
 		item_type: { type: 'number' },
 		sync_disabled: { type: 'number' },
 		sync_disabled_reason: { type: 'string' },
+		sync_warning_ignored: { type: 'number' },
 		sync_target: { type: 'number' },
 		sync_time: { type: 'number' },
 		type_: { type: 'number' },

packages/tools/cspell/dictionary4.txt

@@ -98,4 +98,7 @@ Prec
 ellipsized
 Trashable
 hideable
+unignore
+unignored
+unsyncable
 infini