GraphQL read query's Injector transformation context is way too broad

robbieaverill commented 6 years ago

Affected versions: 3.x and 4.x

The history viewer GraphQL query readOneBlockQuery is applied to a context for ModelAdmin which is far too broad:

  Injector.transform(
    'elements-history',
    (updater) => {
      // Add content block history to the HistoryViewer
      updater.component(
        'HistoryViewer.Form_ItemEditForm', // <--------- context is too broad
        readOneBlockQuery,
        'ElementHistoryViewer'
      );
    }
  );

Essentially this means that the transformation is applied to all GridFields under the "HistoryViewer" context, regardless of the class type. We need to customise this somehow so it's unique for elemental.

The problem happens when you try to add history viewer to another versioned DataObject (other than SiteTree), and elemental's read query is being applied to it instead.

When fixing this issue, please ensure that the example of the same thing in the documentation at https://github.com/silverstripe/silverstripe-framework/pull/8606 is updated so it's also not too broad.

cc @unclecheese

bigSiebs commented 5 years ago

Has any progress been made on this?

robbieaverill commented 5 years ago

Nope. @unclecheese when you find some time it'd be cool to get your input on how we can address this

robbieaverill commented 4 years ago

@unclecheese any ideas about how we could solve this? It's effectively preventing people from using history viewer on anything other than elemental (excluding pages, they're OK) if you're using elemental.

unclecheese commented 4 years ago

Best thing for dataobject history is the versioned-snapshots module and its versioned-snapshot-admin tool, which gives you a generic history viewer anywhere. Just add the snapshot history extension. It’s in the readme.

Sorry don’t have links now, using mobile. 😄

silverstripe / silverstripe-elemental

GraphQL read query's Injector transformation context is way too broad #497