allow for scoped channels #607

akash1810 · 2017-09-18T22:35:55Z

Editorial have experienced problems where they've pasted a link to a video on the main channel, however as its not in the allowedChannel list, updates don't go to YouTube. This results in them editing the Atom and then going to youtube.com to do the same thing. This is time-consuming and not scalable because a lot of the subs don't have yt credentials.

We removed the main channel to prevent too many public uploads.

This PR updates the /api/youtube/channels endpoint, returning a list of privacyStates that a channel can be uploaded to. That is, we can add the main channel to the config youtube.channels.unlisted and allow Editorial to treat this Atom like any other, safe in the knowledge that its always unlisted.

TODO

test on CODE
confirm behaviour with Adam and Katie
update configs in S3 before deploying

this is to add the main channel back in, restricting its upload level

show channel picker in video data disable fields if atom channel not in api channels

akash1810 · 2017-09-20T10:42:19Z

API response before:

[
  {
    "id": "UC-pCzg7zq1Nahs0lENOqyiw",
    "title": "gdn-dev-acc"
  }
]

After:

[
  {
    "id": "UC-pCzg7zq1Nahs0lENOqyiw",
    "title": "gdn-dev-acc",
    "privacyStates": [
      "Unlisted",
      "Private"
    ]
  }
]

prout-bot · 2017-09-20T11:51:21Z

Seen on PROD (merged by @akash1810 22 minutes and 1 second ago) Please check your changes!

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. # Permission to change Privacy Status of a Media Atom In particular, the code around changing 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!

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. # 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: * #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!

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!

akash1810 added 3 commits September 18, 2017 20:53

expand channel api, listing privacyStatus

4f87b9b

this is to add the main channel back in, restricting its upload level

update frontend

022fab7

update client

fca0767

show channel picker in video data disable fields if atom channel not in api channels

akash1810 requested a review from Reettaphant September 19, 2017 10:54

Reettaphant approved these changes Sep 20, 2017

View reviewed changes

akash1810 merged commit ebfda6e into master Sep 20, 2017

akash1810 deleted the aa-channel branch September 20, 2017 11:29

prout-bot added Pending-on-PROD Seen-on-PROD and removed Pending-on-PROD labels Sep 20, 2017

rtyley mentioned this pull request Feb 1, 2024

Upgrade Editorial permissions library to latest version #1142

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

allow for scoped channels #607

allow for scoped channels #607

akash1810 commented Sep 18, 2017 •

edited

Loading

akash1810 commented Sep 20, 2017 •

edited

Loading

prout-bot commented Sep 20, 2017

allow for scoped channels #607

allow for scoped channels #607

Conversation

akash1810 commented Sep 18, 2017 • edited Loading

akash1810 commented Sep 20, 2017 • edited Loading

prout-bot commented Sep 20, 2017

akash1810 commented Sep 18, 2017 •

edited

Loading

akash1810 commented Sep 20, 2017 •

edited

Loading