Add support for "Interest" endpoints, deprecate "Interest label" endpoints

Backwards compatible patch.
The patch marks preference and interest label methods as deprecated (they still work) and adds 3 new API methods:

1. getInterests (returns new InterestCollection object)
2. getContactInterests (returns new InterestCollection object)
3. subscribeContactToInterest

Updated patch with missing doc comment.

I have the following remarks when reviewing:

• The method \Drupal\flexmail\api\value\Value::validateFieldsPresent doesn't seem to be used at the moment.
• It's also a bit strange that helper static methods are placed in an abstract (base) class. Why is that abstract class needed, shouldn't the helper logic be better off in a Utility class?
• Secondary, Maybe ValueBase is a better name, if a base class is deemed useful to add base logic to in a later phase?
• If there is an interface \Drupal\flexmail\api\value\ValueInterface, I'd expect the base class to implement that interface, since based on the name it seems to be something uniform for all value classes.

The logic looks good in general. Could you check the comments above?

Next to that, is there a blog post or more information that can be shared? Since we'll need a follow-up issue to update the flexmail_webform submodule, since there is logic there that depends on the deprecated methods.

Thanks for the review.

• \Drupal\flexmail\api\value\Value::validateFieldsPresent is removed. But keep in mind that the Flexmail module can serve as an API module for other modules (which is the case at District09) and doing so, providing functionality which is not necessarily used internally.
• The abstract class is meant to be used by all other (future) value objects, which all can use its base methods. I don't see the point to make an extra utility class for that. The 2 available methods are static because they are used by another static method in an extending class. The abstract class can contain more common methods in the future applicable to all Flexmail value objects. I made Contact extending ValueBase as well now as it can't break anything.
• Renaming the abstract class: check.
• Of course the abstract class should use the ValueInterface: check.

Abstracted some more code into separate (base) classes/interfaces.
Additionally the 3 existing Value objects are made final.

The Flexmail structural change is shown in a small video on https://app.flexmail.eu/auth. Also when you log in, you should get a warning when your segments are still based on sources instead of interests. Rumour has it it's also communicated through a newsletter.

Thanks for the updates, some feedback again :)

• Why not add sameValueTypeAs and maybe even getId in the interface, so you can use the interface as typehint where appropriate? Or doesn't that make sense?
• You use ValueAbstract::sameValueTypeAs() in the docblock for \Drupal\flexmail\api\value\ValueInterface::sameValueAs, which should be changed to ValueBase::, or if the comment above makes sense and you agree, to ValueInterface::
• It makes sense about keeping the static classes in there, it just looked a bit weird to have only static classes in there, now it just looks more logical in my mind, since there's other regular methods in there.
• In the constructors for some classes a comment says "The constructor is protected", maybe that should be private instead of protected, since the method is private?
• Keeping \Drupal\flexmail\api\value\Value::validateFieldsPresent in there is ok for me, it was just weird to add a method that isn't used. Especially since that's a method that imo should/could be used in the basic API layer. If you feel it's an added value, feel free to add it back in.

Applied all changes. \Drupal\flexmail\api\value\Value::validateFieldsPresent is not kept for now.

Looks good to me, I'll give Matthijs a chance to look at it, if he doesn't have time, I'll merge. ping me if it becomes urgent!

Marking as needs work based on the feedback!

Fixed feedback or added responses to remarks in MR. Marking as 'Needs review'.

Replied in the threads. I would suggest to fix these last 3 issues, then it's RTBC.

We only need a better solution for the getId method, having an empty id method that should never be used for some valueobjects doesn't seem right. Maybe we can have ValueObjectBase and ValueCollectionBase with a common ValueBase? Or using traits as suggested before?

Moved getId to the final value objects that actually (can) have an ID.

For anyone needing a patch, here a version including the latest MR changes.

Updated patch since MR changes.

Updated patch since MR changes.

Issue #3394527: Fix deprecation notice

Merged, thanks for the contribution!

Automatically closed - issue fixed for 2 weeks with no activity.