YSP-672: AI: Add media payloads and pipelines #15
Conversation
While we allow new pages to be indexed by default, it did not make sense to always opt-out on media that is uploaded. This adds the feature that when they're inserting a new media that has ai metatags enabled, it will default the media item to be disabled by default.
Since documents have a different field than images on how they store their associated files, we need a way to retrieve it no matter what.
Instead of placaing the URL inside documentContent, which doesn't quite make sense, place it in the documentUrl. This allows citations to link directly to the PDF, and the edit page it was referencing before really is useless to the users since they won't have access to visit that link anwyay.
If config is nullable we throw an exception anyway, and with new versions of PHP, defaulted paramter arguments must be at the end of the function signature. Instead of moving this, just default it to NULL.
Still work in progress, but would give the ability to "opt-in" or out of AI inclusion hopefully in many different content types.
dblanken-yale
force-pushed
the
YSP-672-ai-add-media-payloads-and-pipelines
branch
from
6cca667
to
4e88876
Compare
dblanken-yale
force-pushed
the
YSP-672-ai-add-media-payloads-and-pipelines
branch
from
ed8a80a
to
2791996
Compare
| protected function isSupportedMediaType(EntityInterface $entity) { | ||
| $config = $this->configFactory->get('ai_engine_embedding.settings'); | ||
| $allowed_media_types = $config->get('included_media_types'); | ||
| return in_array($entity->bundle(), $allowed_media_types); |
There was a problem hiding this comment.
I'm guessing that this will throw an error on sites that already have this module enabled but did not resave the form. Can not check in_array on NULL.
There was a problem hiding this comment.
Ah good catch; I've added ?? [] for $allowed_media_types now.
| @@ -18,6 +20,13 @@ class AiEngineEmbeddingSettings extends ConfigFormBase { | |||
| */ | |||
| const CONFIG_NAME = 'ai_engine_embedding.settings'; | |||
There was a problem hiding this comment.
This is not a part of your current work, but I'm just now realizing that we should have a config/install for this settings file.
There was a problem hiding this comment.
Ah that makes sense. I can add that as part of this.
Thanks to Jim for finding this funny duplciation.
|
@nJim When dealing with an "optional" dependency of a module, is it custom to check for the config the optional module uses or is there another easier way this is done? I wanted to try to combine the metadata enabling with the metatags selection, but I didn't want to couple them since it's possible to run without it. Right now one is in embedding and one is in metadata; I was thinking it's more appropriate in metadata only. |
dblanken-yale
force-pushed
the
YSP-672-ai-add-media-payloads-and-pipelines
branch
2 times, most recently
from
58ca446
to
81ef396
Compare
dblanken-yale
force-pushed
the
YSP-672-ai-add-media-payloads-and-pipelines
branch
from
81ef396
to
90ef0c2
Compare
YSP-672: AI: Add media payloads and pipelines
Description of work
Functional testing steps: