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

added option to prefill break time #591

Merged
merged 5 commits into from
Dec 7, 2020
Merged

Conversation

Flodgar
Copy link

@Flodgar Flodgar commented Nov 24, 2020

Related issue

Closes #355

Context / Background

Added option to automatically prefill break end when punching on break start.
2 new input fields in preferences - Switch that enables this feature and text input for break time length.
Break end will be prefilled only when punching on break start. Manually typing in break start won't prefill the end time.

What change is being introduced by this PR?

Modified punchDate function in BaseCalendar.js to check if we are punching for break start based on index of time input field. Then calculate break end time ( in new function _calculateBreakEnd ) based on user preference and update break end field with this value.

This PR also fixes problem with time input type user preferences not saving/displaying correct value. Problem was with initPreferencesFileIfNotExistsOrInvalid, specifically when validating time inputs with isValidPreferenceTime. This function tries to validate both notification interval and HH:mm time input in one condition. It's confusing and doesn't do what it is supposed to, so I just wrote new function isNotificationInterval and used it properly together with validateTime.
I also removed the function from tests and added tests for isNotificationInterval.

I edited regex in validateTime. Old one allowed up to 29 hours in input.

Added field in translations:

  • enablePrefillBreakTime
  • breakTimeInterval
  • break-time-interval

How will this be tested?

Tested only manually. Will add tests later.

@codecov
Copy link

codecov bot commented Nov 24, 2020

Codecov Report

Merging #591 (9b97a4a) into main (1e9e877) will decrease coverage by 0.57%.
The diff coverage is 32.35%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #591      +/-   ##
==========================================
- Coverage   69.34%   68.76%   -0.58%     
==========================================
  Files          27       27              
  Lines        2107     2132      +25     
  Branches      295      301       +6     
==========================================
+ Hits         1461     1466       +5     
- Misses        579      596      +17     
- Partials       67       70       +3     
Impacted Files Coverage Δ
js/classes/BaseCalendar.js 57.38% <0.00%> (-4.72%) ⬇️
src/preferences.js 75.22% <71.42%> (-0.48%) ⬇️
js/time-math.js 100.00% <100.00%> (ø)
js/user-preferences.js 82.52% <100.00%> (+0.17%) ⬆️

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 1e9e877...9b97a4a. Read the comment docs.

Copy link
Owner

@thamara thamara left a comment

Choose a reason for hiding this comment

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

This is looking pretty great already! I have left a few comments and I'll do some testing and will take another look.

js/classes/BaseCalendar.js Outdated Show resolved Hide resolved
locales/ca-CA/translation.json Outdated Show resolved Hide resolved
src/preferences.html Outdated Show resolved Hide resolved
@ghost
Copy link

ghost commented Nov 27, 2020

Congratulations 🎉. DeepCode analyzed your code in 6.365 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard | Configure the bot

Copy link
Owner

@thamara thamara left a comment

Choose a reason for hiding this comment

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

Looks really good! Thanks!

@thamara
Copy link
Owner

thamara commented Dec 7, 2020

\changelog-update
Message: Enhancement: [#591] You can now set up an automatic break time which will automatically add the next entry when you leave for a break punching the time.
User: Flodgar

@thamara
Copy link
Owner

thamara commented Dec 7, 2020

Thank you again @Flodgar, and sorry for taking so much time to review this.

@thamara thamara merged commit 71cc4ac into thamara:main Dec 7, 2020
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.

Prefill break time (lunch time)
2 participants