Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Start super crude dirty checks for brainstorming #741

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 36 additions & 6 deletions src/actions/org.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { headerWithPath, STATIC_FILE_PREFIX } from '../lib/org_utils';

import sampleCaptureTemplates from '../lib/sample_capture_templates';

import { isAfter, addSeconds } from 'date-fns';
import { isAfter, isEqual, addSeconds } from 'date-fns';
import { parseISO } from 'date-fns';
import { persistIsDirty, saveFileContentsToLocalStorage } from '../util/file_persister';
import { localStorageAvailable, readOpennessState } from '../util/settings_persister';
Expand Down Expand Up @@ -123,7 +123,7 @@ const doSync = ({
return;
}

// Calls do `doSync` are already debounced using a timer, but on big
// Calls to `doSync` are already debounced using a timer, but on big
// Org files or slow connections, it's still possible to have
// concurrent requests to `doSync` which has no merit. When
// `isLoading`, don't trigger another sync in parallel. Instead,
Expand Down Expand Up @@ -152,14 +152,22 @@ const doSync = ({
dispatch(setIsLoading(true, path));
dispatch(setOrgFileErrorMessage(null));

// Before stuff happens asynchronously, remember the current count
// of dirtied actions.
const currentDirtyCount = getState().org.present.getIn(['files', path, 'isDirtyNew', 'id']);

client
.getFileContentsAndMetadata(path)
.then(({ contents, lastModifiedAt }) => {
const isDirty = getState().org.present.getIn(['files', path, 'isDirty']);
const lastServerModifiedAt = parseISO(lastModifiedAt);
const lastSyncAt = getState().org.present.getIn(['files', path, 'lastSyncAt']);

if (isAfter(lastSyncAt, lastServerModifiedAt) || forceAction === 'push') {
if (
isAfter(lastSyncAt, lastServerModifiedAt) ||
isEqual(lastSyncAt, lastServerModifiedAt) ||
forceAction === 'push'
) {
if (isDirty) {
const contents =
localStorage.getItem('files__' + path) ||
Expand All @@ -172,6 +180,7 @@ const doSync = ({
]),
dontIndent: getState().base.get('shouldNotIndentOnExport'),
});

client
.updateFile(path, contents)
.then(() => {
Expand All @@ -180,16 +189,37 @@ const doSync = ({
} else {
setTimeout(() => dispatch(hideLoadingMessage()), 2000);
}
dispatch(setIsLoading(false, path));
dispatch(setDirty(false, path));
dispatch(setLastSyncAt(addSeconds(new Date(), 5), path));

// INFO: This could be made more efficient if
// `updateFile` would already return the new
// `lastModifiedAt` date.
client.getFileContentsAndMetadata(path).then(({ lastModifiedAt }) => {
dispatch(setIsLoading(false, path));
dispatch(setLastSyncAt(parseISO(lastModifiedAt)), path);

// TODO: Get the latest one that actually has isDirty: true.
const newDirtyCount = getState().org.present.getIn([
'files',
path,
'isDirtyNew',
'id',
]);

// Set isDirty to false only if there were no more
// dirtying actions since last we started syncing.
// Otherwise, the flag should be kept.
if (newDirtyCount >= currentDirtyCount) {
dispatch(setDirty(false, path));
}
});
})
.catch((error) => {
const err = `There was an error pushing the file ${path}: ${error.toString()}`;
console.error(err);
dispatch(setDisappearingLoadingMessage(err, 5000));
dispatch(hideLoadingMessage());
dispatch(setIsLoading(false, path));
dispatch(setDirty(true, path));
// Re-enqueue the file to be synchronized again
dispatch(sync({ path }));
});
Expand Down
8 changes: 7 additions & 1 deletion src/reducers/org.js
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,13 @@ const setOpennessState = (state, action) => {
return state.setIn(['opennessState', path], fromJS(opennessState));
};

const setDirty = (state, action) => state.set('isDirty', action.isDirty);
const setDirty = (state, action) => {
const id = _.get(state.get('isDirtyNew'), 'id');
const newId = _.isUndefined(id) ? 0 : id + 1;
state = state.set('isDirty', action.isDirty);
state = state.set('isDirtyNew', { id: newId, isDirty: action.isDirty });
return state;
};

const setSelectedTableCellId = (state, action) => state.set('selectedTableCellId', action.cellId);

Expand Down