BaseModel - Add public getIdValue() method

[najdanovicivan]

Description
This makes it possible to use protected idValue() method from outside of the model to get the id value of the entity.

Checklist:

• Securely signed commits
• Component(s) with PHPdocs
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[MGatner]

It would be great if we could find a way to deprecate the protected version and move towards this instead, but I don't know how to handle that with an abstract method.

[najdanovicivan]

The only way we can introduce deprecation is to make the abstract idValue() method deprecated. Then we change in all places to use getIdValue() instead.

After that we change getIdValue() to also be abstract function and implement in the Model class. the old idValue() method will just pass the call to getIdValue() so for some time we'll have both methods available. In any case it will be breaking change as introducing new abstract method in BaseModel will require the method to be implemented in all child classes

[MGatner]

Understood! Please add a note to the User Guide changelog about this new function - we can handle deduplicating in version 5.

[najdanovicivan]

@MGatner I'll add the deprecation and modify the existing model to use the new method

[najdanovicivan]

If I am to add a new method in the interface it will be BC in any case. Since all the custom models will have to be updated anyway

So for now we can leave the getIdValue() with existing implementation calling the protected idValue() from baseModel(). An override can be added in the model classes to implement the new feature

[najdanovicivan]

I've found a way on how to do deprecation without introducing a BC. The only issue that I have to define both methods in base model.

The only downside of this approach is that any of those methods won't be forced to be defined in the model class. And if there is no single of them defined the infinitive loop will occur.

[MGatner]

I'd rather have the duplicate method than the side effects you notes. Can you revert to abstract and resolve merge conflicts? Then I think this is good.