-
Notifications
You must be signed in to change notification settings - Fork 12
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
Make changes for public organization mediafiles #146
Make changes for public organization mediafiles #146
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only some questions/remarks
@@ -2196,7 +2205,7 @@ list_of_speakers: | |||
- motion_block | |||
- assignment | |||
- topic | |||
- mediafile | |||
- meeting_mediafile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why you can't stay with the mediafile? The permission was checked on assigning the mediafile to an object.
What kind of permission will be checked on usage of the object? If user A has the right to see a mediafile, he can assign the mediafile to a list_of_speakers and to a motion.
If user B is not allowed to see this mediafile, is he forced to see the list_of_speakers without the grafik-icon? On a motion's attachment this may have a sense, on speakers-list not, assuming the mediafile is used as a kind of icon for the speakers list or another object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get what this has to do with permissions, however the reasons I moved all these fields to their own model are:
- To make backend-side meeting-related-mediafile-data-management easier. If, for example, a published orga mediafile's access groups are edited in a meeting, the re-calculation of inherited access groups for this mediafile and its descendants doesn't call for reconsideration of the inherited access groups that are set in any other meetings. It would make no sense at all to juggle endlessly long lists of access groups even though usually only about 3 of them are relevant and may need to be changed at a time.
- To make client-side display easier. For example: If the client wants to display in the mediafile list, whether a mediafile is being projected in this meeting, he shouldn't have to filter through a list of 300 current projections just to see if one of them is from this meeting.
- To keep integrity checks easier. Using the meeting-internal
meeting_mediafile
model allows us to keep theequal_fields: meeting_id
requirement for these relations, which would definitely have to be removed, if we used the basemediafile
model since, due to the new "publishing" feature, it can now have an organization owner, even if it is used in a meeting context. Howevermeeting_mediafile
always has a distinctmeeting_id
Does that answer your questions?
@@ -2393,7 +2402,7 @@ topic: | |||
|
|||
attachment_ids: | |||
type: relation-list | |||
to: mediafile/attachment_ids | |||
to: meeting_mediafile/attachment_ids |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see above
@@ -2673,7 +2682,7 @@ motion: | |||
restriction_mode: C | |||
attachment_ids: | |||
type: relation-list | |||
to: mediafile/attachment_ids | |||
to: meeting_mediafile/attachment_ids |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see above
@@ -3574,7 +3583,7 @@ assignment: | |||
restriction_mode: A | |||
attachment_ids: | |||
type: relation-list | |||
to: mediafile/attachment_ids | |||
to: meeting_mediafile/attachment_ids |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see above
models.yml
Outdated
restriction_mode: A | ||
inherited_access_group_ids: | ||
type: relation-list | ||
to: group/mediafile_inherited_access_group_ids |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think of explaining the calculation rules here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea, will do just that.
Closes #121