Version listing broken because it relied on a GraphQL feature regression

[chillu]

This is high priority, since it is breaking the core functionality of versioned-admin, see https://travis-ci.org/silverstripe/silverstripe-versioned-admin/jobs/477518443. The ReadHistoryViewerPage query errors out with the following:

{"data":null,"errors":[{"message":"Cannot query field \"ID\" on type \"PageWithDescendants\". Did you mean to use an inline fragment on \"Page\", \"SilverStripeErrorPage\", \"SilverStripeTestPage\", \"SilverStripeRedirectorPage\", or \"SilverStripeVirtualPage\"?","locations":[{"line":3,"column":5}]},{"message":"Cannot query field \"Versions\" on type \"PageWithDescendants\". Did you mean to use an inline fragment on \"Page\", \"SilverStripeErrorPage\", \"SilverStripeTestPage\", \"SilverStripeRedirectorPage\", or \"SilverStripeVirtualPage\"?","locations":[{"line":4,"column":5}]}]}

I think this is because the query scaffolder regressed to not using inheritance types, as documented in the README. This was fixed two days ago via merging https://github.com/silverstripe/silverstripe-graphql/pull/176.

Which brings up a problem: The query was written with this regression in place. Now we can no longer query ID directly, we have to get it explicitly from every ancestor and descendant of the class, meaning dynamic construction of the query.

query ReadHistoryViewerPage ($page_id: ID!, $limit: Int!, $offset: Int!) {
  readOnePage(
    Versioning: {
      Mode: LATEST
    },
    ID: $page_id
  ) {
    ID
  ...
}

Aside: This shows the weaknesses of our testing approach. The GraphQL fix broke versioned-admin, but was only noticeable in it's Behat build, which wasn't triggered. The kitchen sink runs nightly, but only unit tests, so doesn't notice these regressions. See https://github.com/silverstripe/cwp-recipe-kitchen-sink/issues/10

[robbieaverill]

It'd be a shame to revert that PR, but I understand the reasoning. The reason we had to architect the history viewer in its current state is because that PR (as well as injectable queries) meant we had no choice. We're also introducing a bunch of React and GraphQL breaking changes in minor release lines (in that people are no longer able to update minor releases and expect their custom/module React/GraphQL implementations to keep working) so perhaps we should update the versioned-admin module and push forwards instead?

[chillu]

Actually, this breaks elemental as well.

[chillu]

I've worked around this by making the "with descendants" logic an opt-in. Leaving it "broken" in admin/graphql because elemental and versioned rely on it. But opting into descendants in my own schema because I can't query fields on subclasses otherwise. https://github.com/open-sausages/silverstripe-graphql/commit/ec2a4110ff354487b44d3d8e0f3d9e221a83e321

The proper fix for this is https://github.com/silverstripe/silverstripe-graphql/issues/209, but maybe the fix above is easy enough to add as a temporary API? I could imagine that lots of people have this issue, since it's very common to rely on DataObject subclasses. And readPages() vs. readRedirectorPages() isn't really a good workaround if I want to act on a list of all pages regardless of their type.

[chillu]

This is fixed now by reverting the above change