Restore original access requirement for scheduled content overview.

Thanks for noticing this, and doing the investigation. I was not directly involved in the conversion to D8 at that time, and although I did post once on that issue, I was not yet using D8 or reviewing the code properly, but I am sorry this fault got through.

I think, rather than remove the permission, we should update the access required for the global list. It may be important for some roles not to see the scheduled content even if they do have 'access content overview'. There may be problems with showing unpublished content and the extra scheduler permission gives admins the further granularity required.

Thanks for working on this.

This seems counterintuitive to the "view scheduled content list"-permission, which states "Allows users to see all content which is scheduled." ... We might want to remove the "extra status" filtering from view's configuration?

Yes, I agree. We are providing only the 'list' of scheduled nodes (which may contain both published and unpublished content), which is what our permission is intended for. Whether or not the user can click the node and read the unpublished content should be controlled by other permissions, and in turn those other permissions should not impact the ability to see the scheduled list. We might want to improve the permission description to make this more clear.

Thank you for the tests, too. I will do some manual testing and also check the D7 behaviour.

I've done manual testing on 8.x, and also checked the 7.x behavior. I have used the following terminology to refer to user roles, and this is the same in both versions:

Notes as numbered [ ] above:

1. Without 'view own unpublished content' the node titles still appears in the list but cannot view the node, as expected.
2. OK but could be improved - but that is a separate issue
3. The page shows all scheduled content (both published and unpublished) for all users. Neither 'Use the administration pages and help' nor 'Access the content overview' are required to access this page. If the user does not have 'Use the administration pages and help' then /admin/ is denied, as is /admin/content, but /admin/content/scheduler is still allowed.

So the patch fixes the five wrong access results, but unfortunately the one which was correct is now wrong. This is because, in ScheduledListAccess the derivation has been copied from 7.x scheduler_list_access_callback. However, in 7.x the parameter $uid is NULL when viewing the full page. This is where the new function gives True. We need to make the distinction in what path is being tested.

I have not yet studied the simpletests. Great work - thank you. I will try to find where exactly the incorrect assertion is letting this pass.

This patch is the same as #4 except with the modified tests, as explaied in #2592293-5: Test access to scheduled node views. There are 13 failed assertions using the committed codebase. I am expecting the changes by mr.baileys in #4 to fix 10 of these failures.

Same code changes as #4 but with finalised expanded tests as per #2592293-9: Test access to scheduled node views. There will be a few failed assertions as before.

As expected, 116 passes and 4 fails.

1. testViewScheduledContentUser
   fail: [Browser] Line 66 of modules/scheduler/src/Tests/SchedulerScheduledContentListAccessTest.php:
   "Scheduler User" cannot access the scheduled content user tab for "Scheduler Manager"

   This is due to giving a user who only has 'schedule publishing of nodes' too much access in ScheduledListAccess. The value of \Drupal::currentUser()->id() == $account->id() is always true. To check if the user is browsing their own page we need \Drupal::routeMatch()->getRawParameter('user') == $account->id()

2. testViewScheduledContentUser
   fail: [Other] Line 81 of modules/scheduler/src/Tests/SchedulerScheduledContentListAccessTest.php:
   "Node created by Scheduler User for publishing" found

   This is where the Manager cannot see the Users unpublished content. As discussed before, this is fixed by removing the 'extra status' filter "published or admin"

3. testViewScheduledContentOverview
   fail: [Browser] Line 103 of modules/scheduler/src/Tests/SchedulerScheduledContentListAccessTest.php:
   "Scheduler User" cannot access the scheduled content overview.
   

   This is caused by the same problem, and solved by the same fix as (1) above

4. testViewScheduledContentOverview
   fail: [Other] Line 111 of modules/scheduler/src/Tests/SchedulerScheduledContentListAccessTest.php:
   "Node created by Scheduler User for publishing" found

   Same as (3) - removing the 'extra status' fixes this

The view code is much shorter because the fields in the overridden display (user tab) were identical to the default, so reverting to default removed hundreds of lines which were not needed :-)

This patch should be fully green with 120 passes.

Thanks @mr.baileys for starting this off. Committed the code here, and the tests on #2592293-10: Test access to scheduled node views

In testing the changed view I had trouble getting the changed config .yml file to be loaded. I was using Config Update Manager module and had to fix #2767635: Scheduled content view has wrong id. Now Config Update Manager is working well and it is easy to export/import/revert views.

I have created #2771375: Use dependency injection for routeMatch() in ScheduledListAccess as a separate issue, so marking this one as fixed.