Skip to content

dev/server-sync-settings#306

Closed
aaayushsingh wants to merge 3 commits intoActivityWatch:masterfrom
aaayushsingh:dev/server-sync-settings
Closed

dev/server-sync-settings#306
aaayushsingh wants to merge 3 commits intoActivityWatch:masterfrom
aaayushsingh:dev/server-sync-settings

Conversation

@aaayushsingh
Copy link
Copy Markdown
Contributor

sync settings to server

@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 29, 2021

Codecov Report

Merging #306 (aed59ea) into master (b27a951) will decrease coverage by 0.20%.
The diff coverage is 0.00%.

❗ Current head aed59ea differs from pull request most recent head 2b6279c. Consider uploading reports for the commit 2b6279c to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #306      +/-   ##
==========================================
- Coverage   15.91%   15.70%   -0.21%     
==========================================
  Files          24       25       +1     
  Lines        1194     1165      -29     
  Branches      134      127       -7     
==========================================
- Hits          190      183       -7     
+ Misses        959      938      -21     
+ Partials       45       44       -1     
Impacted Files Coverage Δ
src/util/settings.ts 0.00% <0.00%> (ø)
src/store/modules/categories.js 65.21% <0.00%> (-6.62%) ⬇️
src/main.js 0.00% <0.00%> (ø)
src/store/modules/activity.ts 6.57% <0.00%> (+0.69%) ⬆️
src/util/timeperiod.ts 18.18% <0.00%> (+0.72%) ⬆️
src/store/modules/views.ts 43.90% <0.00%> (+2.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b27a951...2b6279c. Read the comment docs.

Copy link
Copy Markdown
Member

@ErikBjare ErikBjare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good progress so far, looks like next step is putting it in Vuex and start to refactor some uses of localStorage in the code.

Comment thread .vscode/settings.json
Comment on lines +1 to +3
{
"editor.formatOnSave": true
} No newline at end of file
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can omit this, and add .vscode to .gitignore

Comment on lines +59 to +131
data() {
return {
settingsData: {
durationDefault: '86400',
initialTimestamp: 'Thu Sep 23 2021 15:22:08 GMT+0530',
newReleaseCheckData:
'{"isEnabled":true,"nextCheckTime":"2021-09-29T09:51:36.782Z","howOftenToCheck":86400,"timesChecked":0}',
startOfDay: '04:00',
syncSettingsToServer: true,
userSatisfactionPollData:
'{"isEnabled":true,"nextPollTime":"2021-09-30T09:52:08.703Z","timesPollIsShown":0}',
},
};
},
async created() {
await this.init();
},
methods: {
async init() {
// read data from localstorage
const {
syncSettingsToServer,
startOfDay,
newReleaseCheckData,
initialTimestamp,
durationDefault,
userSatisfactionPollData,
} = localStorage;

// check if its a new user and set sync accordingly
await this.checkNewUser(syncSettingsToServer, durationDefault);

let data;
// if server sync enabled, get settings from server
if (syncSettingsToServer) {
data = await getSettingsFromServer();
} else {
data = {
syncSettingsToServer,
startOfDay,
newReleaseCheckData,
initialTimestamp,
durationDefault,
userSatisfactionPollData,
};
}
this.settingsData = data;
return;
},
async checkNewUser(syncSettingsToServer, durationDefault) {
if (syncSettingsToServer == undefined) {
// sync property doesn't exist
if (durationDefault) {
// existing user, set sync to false by default
localStorage.setItem('syncSettingsToServer', 'false');
} else {
// new user, keep sync on by default
localStorage.setItem('syncSettingsToServer', 'true');
}
}
return;
},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stuff like this should definitely go in a Vuex module.

Comment on lines +55 to +56
this.settingsData.syncSettingsToServer = !this.settingsData.syncSettingsToServer;
updateSettingOnServer('syncSettingsToServer', this.settingsData.syncSettingsToServer);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably need to set the keys one-by-one (unless the API allows something else).

Perhaps the settings keys should be prefixed with setting or config, such that startOfDay becomes config.startOfDay (just in case we want to use the key-value store for something else as well).

added overflow: visible; to VisTimeline.vue to allow tooltip to overflow the container
remove unwanted z-index change from tooltip.js
@ErikBjare ErikBjare force-pushed the dev/server-sync-settings branch from aed59ea to 2b6279c Compare March 15, 2022 07:40
@ErikBjare
Copy link
Copy Markdown
Member

Continued work in #327, closing.

@ErikBjare ErikBjare closed this Mar 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants