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

Fixing preferences derived loading #249

Merged
merged 1 commit into from
May 31, 2020

Conversation

araujoarthur0
Copy link
Collaborator

Context / Background

  • Briefly explain the purpose of the PR by providing a summary of the context

This is fixing an issue in the getDerivedPrefsFromLoadedPrefs method on the user-preferences.js file.
The method was evaluating the loaded preferences in regards to the default ones, but ignoring all the changes it did at the end by returning the wrong variable.
This caused issues when changing the number of keys in the default preferences: for instance, the new key for view in the other PR.

What change is being introduced by this PR?

  • How did you approach this problem?
  • What changes did you make to achieve the goal?
  • What are the indirect and direct consequences of the change?

Returning the correct "derived" variable now.

How will this be tested?

  • How will you verify whether your changes worked as expected once merged?

All tests ran fine. Adding or removing lines from preferences stops it from being reset to default.

@codecov
Copy link

codecov bot commented May 30, 2020

Codecov Report

Merging #249 into master will increase coverage by 0.65%.
The diff coverage is 76.47%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #249      +/-   ##
==========================================
+ Coverage   46.90%   47.56%   +0.65%     
==========================================
  Files          12       12              
  Lines         985      984       -1     
  Branches      177      177              
==========================================
+ Hits          462      468       +6     
+ Misses        458      451       -7     
  Partials       65       65              
Impacted Files Coverage Δ
main.js 0.00% <0.00%> (ø)
js/classes/Calendar.js 66.75% <80.00%> (+1.74%) ⬆️
js/user-preferences.js 87.95% <100.00%> (ø)

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 362f3ca...d128c32. Read the comment docs.

Copy link
Collaborator

@tupaschoal tupaschoal left a comment

Choose a reason for hiding this comment

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

Ah, is this why my preferences never loaded from one start to another? Ship it!

@tupaschoal
Copy link
Collaborator

Ah, remember to edit the changelog.txt

@thamara thamara added this to the v1.5.2 milestone May 31, 2020
@thamara thamara merged commit 7ec5131 into thamara:master May 31, 2020
@thamara
Copy link
Owner

thamara commented May 31, 2020

I included it on the changelog.txt.

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.

3 participants