Optionally enforce a event alarm type #3997

ChristophWurst · 2022-02-25T10:50:30Z

This contributes the second part of #3993.

Admins can enforce DISPLAY or EMAIL as alarm type
The value is used as new default for an reminder
If a reminder matches the enforced type, we hide the radio group all together (there would only be one option after all)
If a reminder doesn't match the enforced type (event created before enforcement or another client) then the user decide to stay on the old value or switch to the enforced one

I will update the Calendar admin docs for this.

occ config:app:set calendar forceEventAlarmType --value=EMAIL

tcitworld · 2022-02-25T11:00:35Z

Admins can enforce DISPLAY or EMAIL as alarm type

Not both, then ?

tcitworld · 2022-02-25T11:00:55Z

lib/Controller/ViewController.php

@@ -93,6 +94,10 @@ public function index():TemplateResponse {
 		$slotDuration = $this->config->getUserValue($this->userId, $this->appName, 'slotDuration', $defaultSlotDuration);
 		$defaultReminder = $this->config->getUserValue($this->userId, $this->appName, 'defaultReminder', $defaultDefaultReminder);
 		$showTasks = $this->config->getUserValue($this->userId, $this->appName, 'showTasks', $defaultShowTasks) === 'yes';
+		$forceEventAlarmType = $this->config->getAppValue($this->appName, 'force_event_alarm_type', '');


camelCase as well

ChristophWurst · 2022-02-25T15:38:21Z

Not both, then ?

That would be equal to no enforcement. Right now you can also only pick between EMAIL/DISPLAY and nothing else unless that was already present in the event.

Any other value set by another client still shows up. This only "enforces" the alarm type for those who only use the Calendar app. Signed-off-by: Christoph Wurst <[email protected]>

codecov · 2022-03-03T17:11:37Z

Codecov Report

Merging #3997 (dfa5e8f) into main (3633e7a) will increase coverage by 0.02%.
The diff coverage is 25.00%.

@@             Coverage Diff              @@
##               main    #3997      +/-   ##
============================================
+ Coverage     29.47%   29.49%   +0.02%     
- Complexity      323      324       +1     
============================================
  Files           220      220              
  Lines          7583     7594      +11     
  Branches       1002     1004       +2     
============================================
+ Hits           2235     2240       +5     
- Misses         5348     5354       +6

Flag	Coverage Δ
javascript	`20.81% <25.00%> (-0.01%)`	⬇️
php	`67.54% <ø> (+0.09%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/components/Editor/Alarm/AlarmList.vue	`0.00% <0.00%> (ø)`
src/components/Editor/Alarm/AlarmListItem.vue	`0.00% <0.00%> (ø)`
src/views/Calendar.vue	`0.00% <ø> (ø)`
src/store/settings.js	`100.00% <100.00%> (ø)`
...ud/apps/calendar/lib/Controller/ViewController.php	`100.00% <0.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 3633e7a...dfa5e8f. Read the comment docs.

max65482 · 2022-03-13T08:35:09Z

By default I cannot select the reminder type anymore (no config switch set). Is this the expected behavior?

Otherwise, I would have expected this condition...

calendar/src/components/Editor/Alarm/AlarmListItem.vue

Lines 269 to 271 in bba3cd5

    
           canChangeAlarmType() { 
        
             return this.forceEventAlarmType !== null && this.alarm.type !== this.forceEventAlarmType 
        
           },

... to be
return this.forceEventAlarmType === null || this.alarm.type !== this.forceEventAlarmType

szaimen · 2022-03-13T08:37:10Z

lib/Controller/ViewController.php

@@ -114,6 +119,7 @@ public function index():TemplateResponse {
 		$this->initialStateService->provideInitialState('show_tasks', $showTasks);
 		$this->initialStateService->provideInitialState('tasks_enabled', $tasksEnabled);
 		$this->initialStateService->provideInitialState('hide_event_export', $hideEventExport);
+		$this->initialStateService->provideInitialState('force_event_alarm_type', $forceEventAlarmType);


Based om the docs is null not allowed as key:
https://github.com/nextcloud/server/blob/3e067edcbe80797a70118f613ac4b752ce512992/lib/public/AppFramework/Services/IInitialState.php#L45

Yes, as mentioned in #4051, this causes log entries.

miaulalala · 2022-03-14T15:01:51Z

Not sure if your occ command is intentionally using forceEventAlarm_type but the initial state checks for forceEventAlarmType just FYI

m-a-v · 2022-03-14T19:00:37Z

But like this you force the reminder type to email. This changes the default behavior where you can choose the alarm type in the UI.

ChristophWurst added the 3. to review Waiting for reviews label Feb 25, 2022

ChristophWurst added this to the v3.2.0 milestone Feb 25, 2022

ChristophWurst requested review from tcitworld, GretaD and szaimen February 25, 2022 10:50

ChristophWurst self-assigned this Feb 25, 2022

tcitworld requested changes Feb 25, 2022

View reviewed changes

ChristophWurst force-pushed the enhancement/optionally-force-event-alarm-type branch from 93f68f3 to d90c27c Compare March 3, 2022 16:08

szaimen requested review from st3iny and removed request for szaimen March 3, 2022 16:09

ChristophWurst mentioned this pull request Mar 3, 2022

Enforce default alarm type for Calendar events nextcloud/documentation#8114

Closed

ChristophWurst force-pushed the enhancement/optionally-force-event-alarm-type branch from d90c27c to 6f66f34 Compare March 3, 2022 16:37

ChristophWurst requested a review from tcitworld March 3, 2022 16:37

ChristophWurst added the enhancement New feature request label Mar 3, 2022

GretaD approved these changes Mar 3, 2022

View reviewed changes

Allow admins to force an event type

dfa5e8f

Any other value set by another client still shows up. This only "enforces" the alarm type for those who only use the Calendar app. Signed-off-by: Christoph Wurst <[email protected]>

ChristophWurst force-pushed the enhancement/optionally-force-event-alarm-type branch from 6f66f34 to dfa5e8f Compare March 3, 2022 17:08

ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Mar 3, 2022

ChristophWurst merged commit 676d119 into main Mar 3, 2022

delete-merged-branch bot deleted the enhancement/optionally-force-event-alarm-type branch March 3, 2022 17:47

ChristophWurst mentioned this pull request Mar 4, 2022

Ability to remove certain menu options on Calendars entries #3993

Closed

klauweg mentioned this pull request Mar 12, 2022

Warning in logfile on each calendar page reload #4051

Closed

szaimen reviewed Mar 13, 2022

View reviewed changes

m-a-v mentioned this pull request Mar 13, 2022

Email notification option disappeared in 3.2.0 #4032

Closed

max65482 mentioned this pull request Mar 14, 2022

Fix alarm type selection if forceAlarmType is null #4060

Merged

miaulalala mentioned this pull request Mar 14, 2022

Inform the user of the enforced alarm type #4063

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optionally enforce a event alarm type #3997

Optionally enforce a event alarm type #3997

ChristophWurst commented Feb 25, 2022 •

edited

Loading

tcitworld commented Feb 25, 2022

tcitworld Feb 25, 2022

ChristophWurst commented Feb 25, 2022

codecov bot commented Mar 3, 2022 •

edited

Loading

max65482 commented Mar 13, 2022

szaimen Mar 13, 2022

max65482 Mar 13, 2022

miaulalala commented Mar 14, 2022

m-a-v commented Mar 14, 2022

Optionally enforce a event alarm type #3997

Optionally enforce a event alarm type #3997

Conversation

ChristophWurst commented Feb 25, 2022 • edited Loading

tcitworld commented Feb 25, 2022

tcitworld Feb 25, 2022

Choose a reason for hiding this comment

ChristophWurst commented Feb 25, 2022

codecov bot commented Mar 3, 2022 • edited Loading

Codecov Report

max65482 commented Mar 13, 2022

szaimen Mar 13, 2022

Choose a reason for hiding this comment

max65482 Mar 13, 2022

Choose a reason for hiding this comment

miaulalala commented Mar 14, 2022

m-a-v commented Mar 14, 2022

ChristophWurst commented Feb 25, 2022 •

edited

Loading

codecov bot commented Mar 3, 2022 •

edited

Loading