Move acl related functions into calACLUtils.jsm

RESOLVED FIXED in 6.2

Status

Calendar
Internal Components
RESOLVED FIXED
a month ago
28 days ago

People

(Reporter: Fallen, Assigned: Fallen)

Tracking

(Blocks: 1 bug)

Lightning 5.7

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

a month ago
Using the script from attachment 8942466 [details] [diff] [review] (which has r+) and the following migrations:

cal.isCalendarWritable             -> cal.acl.isCalendarWritable
cal.userCanAddItemsToCalendar      -> cal.acl.userCanAddItemsToCalendar
cal.userCanDeleteItemsFromCalendar -> cal.acl.userCanDeleteItemsFromCalendar
cal.userCanModifyItem              -> cal.acl.userCanModifyItem

I'd like to move ACL functions out of calUtils.js(m). Again, there will be some manual changes in one patch, and one with all automatic changes. This requires bug 1408662 to be applied to work.
(Assignee)

Comment 1

a month ago
Created attachment 8944386 [details] [diff] [review]
Manual Changes - v1
Attachment #8944386 - Flags: review?(makemyday)
(Assignee)

Comment 2

a month ago
Created attachment 8944387 [details] [diff] [review]
Automatic Changes - v1

Comment 4

a month ago
Comment on attachment 8944386 [details] [diff] [review]
Manual Changes - v1

Review of attachment 8944386 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks, r+ with the formatting nit fixed.

::: calendar/base/modules/calACLUtils.jsm
@@ +32,5 @@
> +     * @return              True if the calendar is writable
> +     */
> +    userCanAddItemsToCalendar: function(aCalendar) {
> +        let aclEntry = aCalendar.aclEntry;
> +        return !aclEntry || !aclEntry.hasAccessControl || aclEntry.userIsOwner || aclEntry.userCanAddItems;

Can you please break this line to meet the max length expectation? Applies also to the two functions below.
Attachment #8944386 - Flags: review?(makemyday) → review+
(Assignee)

Comment 5

29 days ago
Created attachment 8945378 [details] [diff] [review]
Manual Changes - v2

Nit fixed, also re-wrapped a few comments. We're not exactly enforcing the max-len rule, but should consider doing so in the future. I wrapped to 80 now since that is the default in my editor, and it looks slightly better in this file. For the future I'm thinking max-len of 100 makes more sense.
Attachment #8944386 - Attachment is obsolete: true
Attachment #8945378 - Flags: review+
(Assignee)

Comment 6

29 days ago
For checkin, please push both the manual and automatic patch, in that order.
Keywords: checkin-needed

Comment 7

28 days ago
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/3058d4ba1627
Move acl related functions into calACLUtils.jsm - manual changes. r=MakeMyDay
https://hg.mozilla.org/comm-central/rev/d57095f7ae22
Move acl related functions into calACLUtils.jsm - automatic changes. r=MakeMyDay
Status: ASSIGNED → RESOLVED
Last Resolved: 28 days ago
Keywords: checkin-needed
Resolution: --- → FIXED

Updated

28 days ago
Target Milestone: --- → 6.2
You need to log in before you can comment on or make changes to this bug.