Skip to content

Commit 38aa2d1

Browse files
authored
Merge pull request #791 from NotePlan/cursor/reviews-settings-snapshot-rebuild-3af8
Stop unrelated Reviews settings saves rebuilding the project list
2 parents 48988a0 + eb7548a commit 38aa2d1

4 files changed

Lines changed: 196 additions & 16 deletions

File tree

‎jgclark.Reviews/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ See [website documentation for more details](https://noteplan.co/plugins/jgclark
88
- Change: default **Project start string** is now `started` (new installs / missing setting only).
99
- Fix: completing/cancelling outside the current Perspective, or when the note is already gone from `allProjectsList`, no longer blanks or needlessly rebuilds the Rich list; Space names show in the rebuild progress dialog; metadata-term rename is quieter and migrates legacy `#paused` from `project:` to `status:` when status is on.
1010
- dev: Rich list status toggles were empty for `#started` because `allProjectsList` rows lacked `statusHashtag` (so every active project was treated as `#notstarted`). List load backfills `statusHashtag` from notes; finish-review refreshes the note cache before updating the list row.
11+
- Fix: a settings save that does not change project-list scope (including the log regex or log timer, and the first save after a plugin update) no longer rebuilds the project list.
1112

1213
## [2.3.3] - 2026-10-17
1314
- Change: Increase the time between full rebuilds of the Project cache, up to 5 days. (There are quicker refreshes every day.)

‎jgclark.Reviews/src/__tests__/reviewSettings.test.js‎

Lines changed: 132 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,17 @@
11
// @flow
2-
/* globals describe, expect, test */
2+
/* globals describe, expect, test, jest, afterEach */
3+
/* eslint-disable import/first */
34

5+
jest.mock('../../../jgclark.Dashboard/src/perspectiveHelpers', () => ({
6+
getActivePerspectiveDef: jest.fn(),
7+
loadPerspectiveDefsFromPluginSettings: jest.fn(() => Promise.resolve([])),
8+
}))
9+
10+
import { getActivePerspectiveDef, loadPerspectiveDefsFromPluginSettings } from '../../../jgclark.Dashboard/src/perspectiveHelpers'
411
import { METADATA_TERM_SETTINGS_FOR_RENAME } from '../projectStatus'
512
import {
613
getChangedSettingKeys,
14+
getReviewSettings,
715
getSettingsUpdateAction,
816
settingsChangeIsOnlyMetadataTermRenames,
917
} from '../reviewSettings'
@@ -93,6 +101,21 @@ describe('getSettingsUpdateAction', () => {
93101
expect(getSettingsUpdateAction(previous, current)).toBe('none')
94102
})
95103

104+
test('does nothing when only the log regex changes', () => {
105+
const { previous, current } = settingsPair({}, { _logFunctionRE: 'onSettingsUpdated' })
106+
expect(getSettingsUpdateAction(previous, current)).toBe('none')
107+
})
108+
109+
test('does nothing when only the log timer changes', () => {
110+
const { previous, current } = settingsPair({}, { _logTimer: true })
111+
expect(getSettingsUpdateAction(previous, current)).toBe('none')
112+
})
113+
114+
test('rebuilds when a metadata-term setting changes alongside the log regex and timer', () => {
115+
const { previous, current } = settingsPair({}, { startMentionStr: 'begun', _logFunctionRE: '.*', _logTimer: false })
116+
expect(getSettingsUpdateAction(previous, current)).toBe('rebuild')
117+
})
118+
96119
test('does nothing when only another command setting changes', () => {
97120
const { previous, current } = settingsPair({}, {
98121
weeklyProjectProgressBulletSummary: 'Table by sub-folder',
@@ -138,3 +161,111 @@ describe('settingsChangeIsOnlyMetadataTermRenames', () => {
138161
expect(settingsChangeIsOnlyMetadataTermRenames(previous, current, METADATA_TERM_SETTINGS_FOR_RENAME)).toBe(false)
139162
})
140163
})
164+
165+
describe('getReviewSettings', () => {
166+
const store: any = DataStore
167+
const originalLoadJSON = store.loadJSON
168+
const originalSetPreference = store.setPreference
169+
170+
afterEach(() => {
171+
store.loadJSON = originalLoadJSON
172+
store.setPreference = originalSetPreference
173+
})
174+
175+
/**
176+
* @param {Object} overrides
177+
* @returns {Object}
178+
*/
179+
function rawSettings(overrides: Object = {}): { [string]: mixed } {
180+
return {
181+
usePerspectives: false,
182+
foldersToInclude: ['Projects'],
183+
foldersToIgnore: ['Archive'],
184+
includedTeamspaces: ['private'],
185+
startMentionStr: 'started',
186+
completedMentionStr: 'completed',
187+
cancelledMentionStr: 'cancelled',
188+
dueMentionStr: 'due',
189+
reviewIntervalMentionStr: 'review',
190+
reviewedMentionStr: 'reviewed',
191+
nextReviewMentionStr: 'nextReview',
192+
progressStr: 'progress',
193+
projectTypeTags: ['#project'],
194+
...overrides,
195+
}
196+
}
197+
198+
test('does not mutate the object loadJSON returned when overlaying a perspective and filling defaults', async () => {
199+
const loaded = rawSettings({
200+
usePerspectives: true,
201+
useProjectStatus: true,
202+
})
203+
delete loaded.projectMetadataFrontmatterKey
204+
delete loaded.FFlag_UseCacheOfAllPerspectives
205+
delete loaded.notstartedMentionStr
206+
delete loaded.pausedMentionStr
207+
delete loaded.hiddenStatusTags
208+
delete loaded.displayPaused
209+
delete loaded.autoUpdateAfterIdleTime
210+
delete loaded.reviewsTheme
211+
delete loaded.hiddenProjectTypeTags
212+
const before = JSON.parse(JSON.stringify(loaded))
213+
const loadPerspectives: any = loadPerspectiveDefsFromPluginSettings
214+
const getActive: any = getActivePerspectiveDef
215+
loadPerspectives.mockResolvedValue([{ name: 'Work' }])
216+
getActive.mockReturnValue({
217+
name: 'Work',
218+
dashboardSettings: {
219+
includedFolders: 'Work,Home',
220+
excludedFolders: 'Archive',
221+
includedTeamspaces: ['team-1'],
222+
},
223+
})
224+
store.loadJSON = jest.fn(() => Promise.resolve(loaded))
225+
store.setPreference = jest.fn()
226+
227+
const config = await getReviewSettings()
228+
if (config == null) {
229+
throw new Error('getReviewSettings returned null')
230+
}
231+
232+
expect(loaded).toEqual(before)
233+
expect(config).not.toBe(loaded)
234+
expect(config.foldersToInclude).toEqual(['Work', 'Home'])
235+
expect(config.foldersToIgnore).toEqual(['Archive'])
236+
expect(config.includedTeamspaces).toEqual(['team-1'])
237+
expect(config.perspectiveName).toBe('Work')
238+
expect(config.projectMetadataFrontmatterKey).toBe('project')
239+
expect(config.FFlag_UseCacheOfAllPerspectives).toBe(false)
240+
expect(config.notstartedMentionStr).toBe('notstarted')
241+
expect(config.pausedMentionStr).toBe('paused')
242+
expect(config.hiddenStatusTags).toEqual(['#completed', '#cancelled'])
243+
expect(config.displayPaused).toBe(true)
244+
expect(config.hiddenProjectTypeTags).toEqual([])
245+
expect(config.autoUpdateAfterIdleTime).toBe(0)
246+
expect(config.reviewsTheme).toBe('')
247+
expect(store.setPreference).toHaveBeenCalledWith('projectMetadataFrontmatterKey', 'project')
248+
expect(store.setPreference).toHaveBeenCalledWith('useProjectStatus', true)
249+
})
250+
251+
test('does not mutate the object loadJSON returned when perspectives are off', async () => {
252+
const loaded = rawSettings({
253+
usePerspectives: false,
254+
includedTeamspaces: ['team-x'],
255+
})
256+
const before = JSON.parse(JSON.stringify(loaded))
257+
store.loadJSON = jest.fn(() => Promise.resolve(loaded))
258+
store.setPreference = jest.fn()
259+
260+
const config = await getReviewSettings()
261+
if (config == null) {
262+
throw new Error('getReviewSettings returned null')
263+
}
264+
265+
expect(loaded).toEqual(before)
266+
expect(config).not.toBe(loaded)
267+
expect(config.includedTeamspaces).toEqual(['private'])
268+
expect(config.foldersToInclude).toEqual(['Projects'])
269+
expect(loaded.includedTeamspaces).toEqual(['team-x'])
270+
})
271+
})

‎jgclark.Reviews/src/index.js‎

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,25 +3,26 @@
33
//-----------------------------------------------------------------------------
44
// Index for Reviews plugin
55
// by Jonathan Clark
6-
// Last updated 2026-10-16 for v2.4.0 by @jgclark + @CursorAI
6+
// Last updated 2026-10-26 for v2.4.0 by @jgclark + @CursorAI
77
//-----------------------------------------------------------------------------
88

99
// allow changes in plugin.json to trigger recompilation
1010
import pluginJson from '../plugin.json'
1111
import { generateAllProjectsList, recalculateAllProjectsListItems } from './allProjectsListHelpers'
1212
import { migrateAllProjects } from './migration'
1313
import { renderProjectListsIfOpen } from './reviews'
14-
import { METADATA_TERM_SETTINGS_FOR_RENAME } from './projectStatus'
14+
import { getMetadataTermRenames, METADATA_TERM_SETTINGS_FOR_RENAME } from './projectStatus'
1515
import { offerAndRunMetadataTermRenames } from './projectStatusMigrate'
1616
import {
17+
cloneRawSettings,
1718
getLastSettingsSnapshot,
19+
getProjectListRelevantChangedKeys,
1820
getReviewSettings,
1921
getSettingsUpdateAction,
2022
persistLastSettingsSnapshot,
21-
seedLastSettingsSnapshotIfMissing,
2223
settingsChangeIsOnlyMetadataTermRenames,
2324
} from './reviewSettings'
24-
import { JSP, compareObjects, logDebug, logError, logInfo } from '@helpers/dev'
25+
import { JSP, compareObjects, logDebug, logError, logInfo, logWarn } from '@helpers/dev'
2526
import { backupSettings, pluginUpdated, saveSettings, updateSettingData } from '@helpers/NPConfiguration'
2627
import { showMessage, showMessageYesNo } from '@helpers/userInput'
2728

@@ -137,14 +138,18 @@ export async function onSettingsUpdated(): Promise<void> {
137138
// - otherwise do neither
138139
// Only refresh the project list window if it is already open; do not open it from saving settings alone.
139140
try {
140-
const rawSettings: { [string]: any } = await DataStore.loadJSON(`../${pluginID}/settings.json`)
141-
if (rawSettings == null || Object.keys(rawSettings).length === 0) {
141+
const loadedRaw: { [string]: any } = await DataStore.loadJSON(`../${pluginID}/settings.json`)
142+
if (loadedRaw == null || Object.keys(loadedRaw).length === 0) {
142143
throw new Error(`Can't get Review settings. Stopping.`)
143144
}
145+
// Copy before getReviewSettings(), which fills defaults and may overlay perspective
146+
// folders on the object DataStore.loadJSON returned when that object is cached.
147+
const rawSettings: { [string]: any } = cloneRawSettings(loadedRaw)
144148

145149
const previousRaw = getLastSettingsSnapshot()
146150
const action = getSettingsUpdateAction(previousRaw, rawSettings)
147-
logInfo(pluginJson, `Have updated Review settings; action='${action}' (previous snapshot ${previousRaw == null ? 'missing' : 'present'})`)
151+
const changed = getProjectListRelevantChangedKeys(previousRaw, rawSettings)
152+
logInfo(pluginJson, `Have updated Review settings; action='${action}' (previous snapshot ${previousRaw == null ? 'missing' : 'present'}) changed=[${changed.join(',')}]`)
148153

149154
const config = await getReviewSettings()
150155
if (!config) throw new Error(`Can't get Review settings. Stopping.`)
@@ -191,8 +196,24 @@ export async function onUpdateOrInstall(): Promise<void> {
191196
const updateSettingsResult = updateSettingData(pluginJson)
192197
logInfo(pluginID, `- updateSettingData returned code: ${updateSettingsResult}`)
193198

194-
const settingsAfterUpdate = (await DataStore.loadJSON(`../${pluginID}/settings.json`)) || migratedSettings
195-
seedLastSettingsSnapshotIfMissing(settingsAfterUpdate)
199+
const loadedAfterUpdate: { [string]: any } = (await DataStore.loadJSON(`../${pluginID}/settings.json`)) || migratedSettings
200+
// Snapshot a copy of the post-update file. getReviewSettings() may overlay perspective
201+
// folders on a cached loadJSON object, and a pre-update snapshot must not survive
202+
// or the next settings-pane save replays that diff as a rebuild.
203+
const settingsAfterUpdate = cloneRawSettings(loadedAfterUpdate)
204+
const previousRaw = getLastSettingsSnapshot()
205+
if (previousRaw != null) {
206+
const renames = getMetadataTermRenames(previousRaw, settingsAfterUpdate)
207+
if (renames.length > 0) {
208+
const config = await getReviewSettings()
209+
if (config) {
210+
await offerAndRunMetadataTermRenames(previousRaw, settingsAfterUpdate, config)
211+
} else {
212+
logWarn(pluginID, `onUpdateOrInstall: metadata terms changed but Review settings could not be loaded, so no rename was offered`)
213+
}
214+
}
215+
}
216+
persistLastSettingsSnapshot(settingsAfterUpdate)
196217

197218
// Tell user the plugin has been updated
198219
await pluginUpdated(pluginJson, { code: updateSettingsResult, message: 'Plugin Installed or Updated.' })

‎jgclark.Reviews/src/reviewSettings.js‎

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
// Settings for the Reviews plugin: load/normalise config, heading-setting parse,
44
// and what a settings-pane save should do to the project list.
55
// by @jgclark
6-
// Last updated 2026-10-05 for v2.3.1 by @jgclark + @CursorAI
6+
// Last updated 2026-10-26 for v2.4.0 by @jgclark + @CursorAI
77
//-----------------------------------------------------------------------------
88

99
import { getActivePerspectiveDef, loadPerspectiveDefsFromPluginSettings } from '../../jgclark.Dashboard/src/perspectiveHelpers'
@@ -118,7 +118,7 @@ export const SETTINGS_THAT_REQUIRE_RECALCULATE: $ReadOnlyArray<string> = [
118118
]
119119

120120
/**
121-
* Settings for commands other than the project list (weekly progress, complete/cancel, pause, next review, progress lines, log level).
121+
* Settings for commands other than the project list (weekly progress, complete/cancel, pause, next review, progress lines, log level, log regex, and log timer).
122122
* Changing only these must not rebuild, recalculate, or redisplay allProjectsList.
123123
*/
124124
export const SETTINGS_FOR_OTHER_COMMANDS: $ReadOnlyArray<string> = [
@@ -133,6 +133,8 @@ export const SETTINGS_FOR_OTHER_COMMANDS: $ReadOnlyArray<string> = [
133133
'weeklyProjectProgressBulletSummary',
134134
'weeklyProjectProgressShowEmptyFolders',
135135
'_logLevel',
136+
'_logFunctionRE',
137+
'_logTimer',
136138
]
137139

138140
/**
@@ -226,11 +228,11 @@ export async function getReviewSettings(externalCall: boolean = false): Promise<
226228
logInfo(pluginJson, `getReviewSettings() Starting from a different plugin ...`)
227229
}
228230
// Get settings
229-
const config: ReviewConfig = await DataStore.loadJSON('../jgclark.Reviews/settings.json')
231+
const loaded: ReviewConfig = await DataStore.loadJSON('../jgclark.Reviews/settings.json')
230232

231233
// If an external call allow silent return of null if no settings found.
232234
// Otherwise complain, as there should be settings.
233-
if (config == null || Object.keys(config).length === 0) {
235+
if (loaded == null || Object.keys(loaded).length === 0) {
234236
if (externalCall) {
235237
// Fail silently
236238
return null
@@ -239,6 +241,10 @@ export async function getReviewSettings(externalCall: boolean = false): Promise<
239241
throw new Error('No Reviews settings found')
240242
}
241243
// clo(config, `Review settings for '${pluginJson['plugin.version']}' version:`)
244+
// Clone before any default fill or perspective overlay. DataStore.loadJSON may
245+
// return a cached object; mutating it makes the next settings-pane save look
246+
// like a folder or scope change.
247+
const config: ReviewConfig = (cloneRawSettings((loaded: any)): any)
242248

243249
// Need to store some things in the Preferences API mechanism, in order to pass things to the Project class
244250
DataStore.setPreference('startMentionStr', config.startMentionStr)
@@ -376,6 +382,27 @@ export function getChangedSettingKeys(previous: ?{ [string]: any }, current: ?{
376382
return changed
377383
}
378384

385+
/**
386+
* Deep-copy a plain settings object so later default fills and perspective overlays
387+
* cannot mutate the object DataStore.loadJSON returned.
388+
* @param {{ [string]: any }} settings
389+
* @returns {{ [string]: any }}
390+
*/
391+
export function cloneRawSettings(settings: { [string]: any }): { [string]: any } {
392+
return (JSON.parse(JSON.stringify(settings)): any)
393+
}
394+
395+
/**
396+
* Keys that differ, ignoring SETTINGS_FOR_OTHER_COMMANDS (log level, log regex, log timer, and other non-list commands).
397+
* Empty when either side is missing.
398+
* @param {?{ [string]: any }} previous
399+
* @param {?{ [string]: any }} current
400+
* @returns {Array<string>}
401+
*/
402+
export function getProjectListRelevantChangedKeys(previous: ?{ [string]: any }, current: ?{ [string]: any }): Array<string> {
403+
return getChangedSettingKeys(previous, current).filter((key) => !SETTINGS_FOR_OTHER_COMMANDS.includes(key))
404+
}
405+
379406
/**
380407
* Decide what a settings-pane save should do to the project list.
381408
* Priority: rebuild (membership/metadata) > recalculate (existing rows) > redisplay (render only) > none.
@@ -388,7 +415,7 @@ export function getSettingsUpdateAction(previous: ?{ [string]: any }, current: ?
388415
if (previous == null || current == null) {
389416
return 'rebuild'
390417
}
391-
const changed = getChangedSettingKeys(previous, current).filter((key) => !SETTINGS_FOR_OTHER_COMMANDS.includes(key))
418+
const changed = getProjectListRelevantChangedKeys(previous, current)
392419
if (changed.some((key) => SETTINGS_THAT_REQUIRE_REBUILD.includes(key))) {
393420
return 'rebuild'
394421
}
@@ -416,7 +443,7 @@ export function settingsChangeIsOnlyMetadataTermRenames(
416443
metadataTermSettingKeys: $ReadOnlyArray<string>,
417444
): boolean {
418445
if (previous == null || current == null) return false
419-
const changed = getChangedSettingKeys(previous, current).filter((key) => !SETTINGS_FOR_OTHER_COMMANDS.includes(key))
446+
const changed = getProjectListRelevantChangedKeys(previous, current)
420447
if (changed.length === 0) return false
421448
return changed.every((key) => metadataTermSettingKeys.includes(key))
422449
}

0 commit comments

Comments
 (0)