Support from Acquia helps fund testing for Drupal Acquia logo

Comments

josephdpurcell created an issue. See original summary.

josephdpurcell’s picture

Here is a quick initial concept of how this might work just to get early feedback. I think at this point it will make sense to have a test on the Changes class.

josephdpurcell’s picture

uuids are strings!

This patch takes the approach that the doc ids filter does not leverage the same structure as other filters would.

Grayside’s picture

I would like to test this functionality, what needs to happen before that?

josephdpurcell’s picture

Hey @grayside, I've completely rewritten the patch to include generic filters. I've created a PR on GitHub just for testing the work and plan to create a patch here sometime later this week: https://github.com/relaxedws/drupal-replication/pull/6

josephdpurcell’s picture

Title: Support "doc_ids" filter » Support repllication filters
Issue summary: View changes

Updating the description to be generic.

josephdpurcell’s picture

There are two pull requests that go hand-in-hand, one for the Replication module (https://github.com/relaxedws/drupal-replication/pull/6) and the other for the Workspace module (https://github.com/timmillwood/drupal-workspace/pull/5). This ticket is for reviewing the Replication module PR, however we should validate that the ReplicationTaskInterface will work well for any ReplicatorInterface, such as in the Workspace module.

Here are a couple important notes about implementation:

  1. The exact logic for the UUIDs filter is here: https://github.com/relaxedws/drupal-replication/pull/6/files#diff-e3e1b4..., it is activated simply by setting the UUIDs. The benefit of this approach is that you can filter by UUIDs without having to load each document, see here: https://github.com/relaxedws/drupal-replication/pull/6/files#diff-e3e1b4.... This differs from the requested approach which is UUIDs must be used with the UUIDs filter and therefore each document will be loaded.
  2. There are two replication filters implemented: a PublishedFilter https://github.com/relaxedws/drupal-replication/pull/6/files#diff-fcae0d..., and an EntityType filter https://github.com/relaxedws/drupal-replication/pull/6/files#diff-d6f7cc...
  3. I was uncertain whether ReplicationTask should be passed to the "applies" method of a ReplicatorInterface, see https://github.com/timmillwood/drupal-workspace/pull/5/files#diff-76a58c...
josephdpurcell’s picture

This latest patch removes "uuids" from Changes class and ReplicationTask class and makes it a parameter. So, to activate the UUID filter you would pass in filter=uuid and uuids=123,456.

josephdpurcell’s picture

This patch extends #8 by automatically setting the filter to "uuid" if the "uuids" parameter is passed.

josephdpurcell’s picture

Status: Active » Needs review

  • jeqq authored 35461d4 on 8.x-1.x
    git commit -m 'Issue #2710947 by josephdpurcell: Support replication...
jeqq’s picture

Status: Needs review » Fixed

Thank you @josephdpurcell!

JamesK’s picture

Title: Support repllication filters » Support replication filters

@josephdpurcell this made my day! Awesome work!

The last submitted patch, 2: 2710947.patch, failed testing.

The last submitted patch, 2: 2710947.patch, failed testing.

The last submitted patch, 2: 2710947.patch, failed testing.

The last submitted patch, 3: 2710947_3.patch, failed testing.

The last submitted patch, 3: 2710947_3.patch, failed testing.

The last submitted patch, 3: 2710947_3.patch, failed testing.

The last submitted patch, 8: 2710947_8.patch, failed testing.

The last submitted patch, 8: 2710947_8.patch, failed testing.

The last submitted patch, 8: 2710947_8.patch, failed testing.

Status: Fixed » Needs work

The last submitted patch, 9: 2710947_9.patch, failed testing.

The last submitted patch, 9: 2710947_9.patch, failed testing.

The last submitted patch, 9: 2710947_9.patch, failed testing.

jeqq’s picture

Status: Needs work » Closed (fixed)