FieldMapperBase not supporting programatically declared properties

Hi!
I reviewed the changes. Looks quite good so far and I agree on the change to get mappable fields from the xntt. It makes sens.
However, I less confident in this part you removed:

      $derived_entity_type = $this
        ->getExternalEntityType()
        ->getDerivedEntityType();
      if (empty($derived_entity_type)) {
        $this->logger->warning('FieldMapperBase::getFieldDefinition(): Cannot get external entity derived type.');
        return NULL;
      }

This is used by annotation entities. I did not make automated tests on these so I'm not sure it will work properly. I believe you removed that part also because it was not working with your use case right?
What about keeping that part but commented then?

      /*
      @todo The following code as been disabled and impacts on annotation nodes should be checked before removing it permanently.
      @code
      $derived_entity_type = $this
        ->getExternalEntityType()
        ->getDerivedEntityType();
      if (empty($derived_entity_type)) {
        $this->logger->warning('FieldMapperBase::getFieldDefinition(): Cannot get external entity derived type.');
        return NULL;
      }
      @endcode
      */

...or something similar.

@guignonv Thanks for the feedback, much appreciated!

I removed it because with the new code $derived_entity_type isn't used anymore and pretty much the same code is used in the concrete implementation of ConfigurableExternalEntityTypeInterface::getMappableFields() -
ExternalEntityType::getMappableFields:getMappableFields():

      $derived_entity_type = $this->getDerivedEntityType();

      $fields = [];
      if (!empty($derived_entity_type)) {
        $fields = $this
          ->entityFieldManager()
          ->getFieldDefinitions(
            $derived_entity_type->id(),
            $derived_entity_type->id()
          );
      }

It just also fires the event.

If we want the error logging we might want to move it to ExternalEntityType::getMappableFields:getMappableFields()?
But annotation entities support should be the same, at least in this concrete implementation.
If we do not want to rely on the concrete implementation we might need to add another interface so that concrete implementations signal that they support annotation.

Fair enough! :) Merged! (I got time to update the repo now)

@guignonv Awesome! Have a fantastic weekend!

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