Problem/Motivation

Workspace module is being built in a way to support multiple types of replication, including to and from remote Drupal sites. However core will only ever implement local replication. Therefore we need a test to make sure the local replication implementation will still allow for contrib to add remote replication.

Proposed resolution

- Add workspace_remote_test test module.
- Add a test to replicate back to the local site over http.

Remaining tasks

User interface changes

API changes

Data model changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

timmillwood created an issue. See original summary.

timmillwood’s picture

Using a feature branch workspace_remote_test to flesh out the test module before converting to a patch.
https://cgit.drupalcode.org/workspace/log/?h=workspace_remote_test

timmillwood’s picture

Status: Active » Needs review
FileSize
13.75 KB

initial patch.

Status: Needs review » Needs work

The last submitted patch, 3: 2926472-3.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

timmillwood’s picture

Status: Needs work » Needs review
FileSize
1018 bytes
13.75 KB

Ah ha! 403, not 404.

timmillwood’s picture

FileSize
8.22 KB
14.96 KB

Fixing coding standards issues.
Removing the remote item on the upstream plugin.

Status: Needs review » Needs work

The last submitted patch, 6: 2926472-6.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

amateescu’s picture

Status: Needs work » Needs review

Here's what I was proposing in our chat: #2927248: Make upstream plugins non-configurable by default and improve their integration with the Workspace entity , and why I think the 'remote' annotation key is quite useful.

Given the issue above, this test needs to go through the entire flow:

- go to the workspace add form
- select the test remote upstream plugin
- check that the configuration form appears
- fill in values for the endpoint URL
- check that the values were correctly saved in the database by instantiating the plugin with $upstream = $workspace->getUpstreamPlugin() and looking at $upstream->getConfiguration()

timmillwood’s picture

Assigned: timmillwood » Unassigned

Note: now there's a patch here I've deleted the branch workspace_remote_test so the link in #2 won't work.

Wim Leers’s picture

Issue tags: +API-First Initiative
  1. +++ b/tests/modules/workspace_remote_test/src/Plugin/rest/resource/ReplicationResource.php
    @@ -0,0 +1,115 @@
    + *     "create" = "/admin/config/workflow/workspace/{workspace}/replication"
    ...
    +  public function post($workspace_id, $revisions) {
    

    This does not match.

    If the parameter would be WorkspaceInterface $workspace, then it'd automatically receive the entity.

  2. +++ b/tests/modules/workspace_remote_test/src/Plugin/rest/resource/ReplicationResource.php
    @@ -0,0 +1,115 @@
    +    $this->serializer = $serializer;
    ...
    +        $entity = $this->serializer->deserialize(json_encode($revision), $class, 'json');
    

    All of this should not be necessary, \Drupal\rest\RequestHandler::handle() takes care of this.

  3. +++ b/tests/modules/workspace_remote_test/src/Plugin/rest/resource/ReplicationResource.php
    @@ -0,0 +1,115 @@
    +    return new ResourceResponse(TRUE);
    

    This … does not make sense. See \Drupal\rest\Plugin\rest\resource\EntityResource::post(). It should return a ModifiedResourceResponse containing the created data, or at least links to the created data.

timmillwood’s picture

FileSize
3.55 KB
14 KB

❤ Thank you @Wim Leers!

Here's a quick patch just to fix the broken test in #6 and add remote annotation back as it looks like we might need it for #2927248: Make upstream plugins non-configurable by default and improve their integration with the Workspace entity.

timmillwood’s picture

After talking to @WimLeers I've started looking into using REST modules resources rather than creating a custom one.

timmillwood’s picture

FileSize
1.65 KB
14.14 KB

I think using the core REST resources for this would not work, so sticking with the slightly hacky solution for now. I was getting issues with Access denied on creating field for various fields such as nid, revision_uid, changed, status, etc.

Looking to implement suggestions from #10.

  • #10.1 is giving me the following error:
    1) Drupal\Tests\workspace\Functional\RemoteReplicationTest::testRemoteReplication
    Exception: Warning: ReflectionClass::isInstance() expects parameter 1 to be object, array given
    Drupal\Component\Utility\ArgumentsResolver->getArgument()() (Line: 88)
    
  • Fixing #10.2 would mean adding a custom serializer, I assume, because we're sending an array of entity revisions, where each revision may be from a different entity type. Or we could send each revision in a separate request.
  • #10.3 shouldn't be an issue, patch attached for that.
amateescu’s picture

Project: Workspace » Drupal core
Version: 8.x-2.x-dev » 8.6.x-dev
Component: Code » workspace.module
Status: Needs review » Needs work
Issue tags: +Workflow Initiative

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

amateescu’s picture

Component: workspace.module » workspaces.module

Fix component following module rename.

amateescu’s picture

Project: Drupal core » Workspace
Version: 8.7.x-dev » 8.x-2.x-dev
Component: workspaces.module » Code

Remote replication is not in the scope of the core Workspaces module anymore, so moving back to the contrib queue for now.