Upgrade Editorial permissions library to latest version #1142
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rtyley
force-pushed
the
upgrade-permissions-library
branch
from
4fdc945 to
b08dcb3
Compare
rtyley
changed the title
rtyley
force-pushed
the
upgrade-permissions-library
branch
5 times, most recently
from
b49e3b0 to
8316075
Compare
rtyley
force-pushed
the
upgrade-permissions-library
branch
from
8316075 to
9abd55e
Compare
Merged
rtyley
force-pushed
the
upgrade-permissions-library
branch
2 times, most recently
from
9cd7d90 to
a682f9b
Compare
rhystmills
approved these changes
Feb 8, 2024
This upgrades the Media Atom Maker to use the latest version of the client for the Guardian's Editorial Permissions service - we need the latest version of the client to support the upgrade to Scala 2.13 in #1140 * Before: https://github.com/guardian/editorial-permissions-client/tree/v0.8 - supporting Scala 2.11 & 2.12 * After: https://github.com/guardian/permissions/tree/v2.15/client - supporting Scala 2.12 & 2.13 As you can see, the permissions client has moved repositories, to the main `permissions` repo - this happened in July 2018 with PR guardian/permissions#103. This PR is also important because it removed use of `Future` from the permissions client API - as Michael Barton explained, permission lookups should be mostly instantaneous because they now come from an in-memory cache. The removal of `Future` means that this commit, upgrading permissions in Media Atom Maker, needs to remove several for-comprehensions/map-statements. The diff on these can look quite big, but they look smaller if whitespace changes are ignored. I have taken the opportunity to do small refactors to improve code clarity and remove repetition. In particular, the code around modifying Privacy Status of a Media Atom _had_ to be changed because it involved removing `Future`, but I also included refactoring to make the code clearer. When reviewing this, you may want to look at the original PRs that introduced this logic: * #607 - introduced the concept of each of our YouTube channels having a different set of available PrivacyStatus (Private, Unlisted, Public) values. * #694 - you can always upload as Public unless the channel is in the youtube.channels.unlisted config, in which case you need permission. This means we can give general users the ability to upload as Public on the culture channel and grant specific people access to make a public video on the main channel. * #789 - public video should *stay* public when a metadata change is made by someone who does not have permission to *make* a video public on that channel. * #791 - code shouldn't fail if the atom has not been published yet!
rtyley
force-pushed
the
upgrade-permissions-library
branch
from
a682f9b to
a0b063b
Compare
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This upgrades the Media Atom Maker to use the latest version of the client for the Guardian's Editorial Permissions service - we need the latest version of the client to support the upgrade to Scala 2.13 in #1140
Permissions library versions
As you can see, the permissions client has moved repositories, to the main
permissionsrepo - this happened in July 2018 with PR https://github.com/guardian/permissions/pull/103. This PR is also important because it removed use ofFuturefrom the permissions client API - as Michael Barton explained, permission lookups should be mostly instantaneous because they now come from an in-memory cache.The removal of
Futuremeans that this PR, upgrading Media Atom Maker, needs to remove several for-comprehensions/map-statements. The diff on these can look quite big, but they look smaller if whitespacechanges are ignored. I've taken the opportunity to do small refactors to improve code clarity and remove repetition.
Permission to modify Privacy Status of a published Media Atom
In particular, the code around modifying Privacy Status of a Media Atom had to be changed because it involved removing
Future, but I also included refactoring to make the code clearer. When reviewing this, you may want to look at the original PRs that introduced this logic:youtube.channels.unlistedconfig, in which case you need permission. This means we can give general users the ability to upload as Public on the culture channel and grant specific people access to make a public video on the main channel.