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.

Sign up for GitHub

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

Jump to bottom

Added byday recurrence feature #918

Merged
merged 1 commit into from
Jun 11, 2018
Merged

Added byday recurrence feature #918

merged 1 commit into from
Jun 11, 2018

Conversation

Julian1998
Copy link
Contributor

Instructions:

  1. create a new event
  2. click on more...
  3. go to repeating
  4. create a new recurrence with byday options

Possible scenarios:

  1. creating a new event with byday options
  2. modifying the current byday options

All events should render correctly even when changing the first day of the week or/and language

All scenarios work fine in Chrome

DeepDiver1975
DeepDiver1975 reviewed May 19, 2018
View reviewed changes
js/app/controllers/recurrencecontroller.js Outdated
});
ctrl.transferDaysToByDay = function(newDays) {
angular.forEach(newDays, function(value, key) {
if(typeof $scope.properties.rrule.byday === 'undefined') {
Copy link
Member

Choose a reason for hiding this comment

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

angular.isUndefined

DeepDiver1975
DeepDiver1975 requested changes May 30, 2018
View reviewed changes
js/app/controllers/recurrencecontroller.js Outdated
@@ -64,7 +64,7 @@ app.controller('RecurrenceController', function($scope) {
$scope.rruleNotSupported = true;
}

if (typeof $scope.properties.rrule.parameters !== 'undefined') {
if (!angular.isUndefined($scope.properties.rrule.parameters)) {
Copy link
Member

Choose a reason for hiding this comment

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

dont use inverted logic - https://docs.angularjs.org/api/ng/function/angular.isDefined

@DeepDiver1975
Copy link
Member

DeepDiver1975 commented May 30, 2018 •
edited
Loading

bildschirmfoto von 2018-05-30 19-15-35

jslint hates you ;-)

@DeepDiver1975
Copy link
Member

The day picker is not working for me ....

@Julian1998
Copy link
Contributor Author

jslint didn't tell me... alright I will have a look at it... Which day picker exactly?

@DeepDiver1975
Copy link
Member

Which day picker exactly?

the monday, tueseday and so on one .... will show you these days live ...

@Julian1998
Copy link
Contributor Author

Seems like jslint didn't build the js sources correctly because of this error. I will fix it and then it should build

@DeepDiver1975
Copy link
Member

@Julian1998 ping

@codecov-io
Copy link

codecov-io commented Jun 7, 2018 •
edited
Loading

Codecov Report

Merging #918 into master will decrease coverage by 0.42%.
The diff coverage is 6.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #918      +/-   ##
==========================================
- Coverage   58.55%   58.12%   -0.43%     
==========================================
  Files          66       66              
  Lines        3274     3305      +31     
==========================================
+ Hits         1917     1921       +4     
- Misses       1357     1384      +27
Impacted Files Coverage Δ
js/app/controllers/recurrencecontroller.js 1.72% <0%> (-1.31%) ⬇️
js/app/models/simpleEventModel.js 87.95% <50%> (-0.52%) ⬇️
controller/settingscontroller.php 78.29% <0%> (+0.34%) ⬆️

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 e021af0...9cf3a67. Read the comment docs.

@coveralls
Copy link

coveralls commented Jun 7, 2018 •
edited
Loading

Coverage Status

Coverage decreased (-0.5%) to 54.269% when pulling 9cf3a67 on byDayRule into e021af0 on master.

@DeepDiver1975
Copy link
Member

looks like tests are failing

Firefox 60.0.0 (Linux 0.0.0) The SimpleEvent factory should add the repeating information FAILED
	TypeError: newSimpleData.rrule.parameters is undefined in js/app/models/simpleEventModel.js (line 9)
	repeating@js/app/models/simpleEventModel.js:9:32091
	SimpleEvent/iface.patch@js/app/models/simpleEventModel.js:9:34974
	@tests/js/unit/models/simpleEventModelSpec.js:2156:3
	<Jasmine>
Firefox 60.0.0 (Linux 0.0.0): Executed 161 of 377 (1 FAILED) (0 secs / 0 secs)
 (1 FAILED) (0 secs / 0 secs)

@DeepDiver1975 DeepDiver1975 merged commit 8494037 into master Jun 11, 2018
@DeepDiver1975 DeepDiver1975 deleted the byDayRule branch June 11, 2018 08:14
@DeepDiver1975 DeepDiver1975 mentioned this pull request Jun 12, 2018
Closed
@DeepDiver1975 DeepDiver1975 mentioned this pull request Sep 19, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DeepDiver1975 DeepDiver1975 DeepDiver1975 approved these changes

@felixboehm felixboehm Awaiting requested review from felixboehm

@felixheidecke felixheidecke Awaiting requested review from felixheidecke

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Julian1998 @DeepDiver1975 @codecov-io @coveralls