Skip to content

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 TTLApp:master May 31, 2020
@thamara
Copy link
Collaborator

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