Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reimplement Matcher with plugin instances support #12

Open
mirodietiker opened this issue Jul 27, 2015 · 2 comments
Open

Reimplement Matcher with plugin instances support #12

mirodietiker opened this issue Jul 27, 2015 · 2 comments

Comments

@mirodietiker
Copy link
Member

The Matchers currently don't support instances and all forms just work for the default instances.

This needs a proper port to D8 plugin concept.

@mbovan
Copy link
Contributor

mbovan commented Aug 3, 2015

I pushed changes that I made in the feature branch (feature/crm-core-arch-changes):
Diff with CRM Core 8.x-0.x branch.

Here are the patches to apply on Collect and Inmail so it's easier to manually test changes by enabling collect_demo:

I'll try to summary changes from local commit messages:

  • MatchingRule from crm_core_default_matching_engine becomes Matcher config entity in crm_core_match module
  • MatchEngineBase inherits PluginBase, MatchEngineInterface extends PluginInspectionInterface and PluginFormInterface
  • DefaultMatchingEngine becomes plugin (plugin_id property) of a Matcher
  • Removed unused routes, changed existing to follow D8 standards
  • MatcherListBuilder to show all created matchers.
  • Forms for adding/editing/deleting a Matcher.
  • Implemented dynamic schema for plugins (match engines).
  • Due to new changes, matcher settings doesn't support selection of the fields per contact type bundle. Instead, fields from all bundles are displayed. (We agreed that this should be a follow-up)
  • Removed status property from a Matcher - no use case for it.
  • Several UI changes, matcher overview, matcher edit form (removed empty operator option for field matching selection)
  • Added some todos for hard-coded matcher calls (EventSubscriber and ContactMatcher in Collect)

There is some commented-out (Drupal 7?) code (e.g. in crm_core_match.test.inc) we should decide what to do with. Seems that crm_core_match\Matcher and crm_core_match\MatcherInterface are not used anymore but I didn't delete them as there are some methods that maybe we should move/implement to/in other classes.

Regarding tests, I didn't run any automatic test (tested manually), I guess there are lot of fails, especially for Unit/kernel tests, but the purpose of the first patch is to show an idea where we want to be at the end. :)

Waiting for review... @mirodietiker, @Berdir

Screenshot of Matchers overview page
Matchers Overview page

The form for adding a new matcher
Form for adding a new matcher

@mirodietiker
Copy link
Member Author

Committed a followup from the plubin instances for collect integration: #21

Can we still close this issue or leave it open until #17 is fixed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants