Add a mapping target to media field

Created on 6 December 2017, almost 7 years ago
Updated 19 June 2023, over 1 year ago

Hello,

I want to import datas from a rss feed to generate medias (facebook).
But, I don't see the "media target" in the settings of my feed type.
Do you think add this target in the futur?

Thanks.

โœจ Feature request
Status

Needs work

Version

3.0

Component

Code

Created by

๐Ÿ‡ซ๐Ÿ‡ทFrance kumkum29

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • ๐Ÿ‡จ๐Ÿ‡ทCosta Rica Royden_CH

    Compatible with core 9.5.9 and 8.x-3.0-beta4

  • Open in Jenkins โ†’ Open on Drupal.org โ†’
    Core: 9.5.5 + Environment: PHP 8.1 & pgsql-10.12
    last update over 1 year ago
    690 pass, 5 fail
  • Open in Jenkins โ†’ Open on Drupal.org โ†’
    Core: 9.5.x + Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    693 pass, 4 fail
  • ๐Ÿ‡ต๐Ÿ‡ฑPoland besek

    #80 worked for me, however it saevd node title as image alt. I think it should be configurable so you can choose what values you want to save in title and alt fields of the image. Thanks

  • ๐Ÿ‡ต๐Ÿ‡นPortugal jrochate

    I don't have standard media machine names, and the latest path WSODs.

    Drupal\Component\Plugin\Exception\PluginNotFoundException: The "media_base_image" plugin does not exist. Valid plugin IDs for Drupal\feeds\Plugin\Type\FeedsPluginManager are: file, datetime, number, media_file, uri, feeds_item, config_entity_reference, integer, temporary_target, daterange, email, timestamp, password, book, image, media_image, user_role, link, text, boolean, telephone, langcode, entity_reference, path, string, geofield_feeds_target, paragraphs in Drupal\Core\Plugin\DefaultPluginManager->doGetDefinition() (line 53

    and also

    Error: Class "Drupal\feeds\Feeds\Target\MediaFile" not found in Drupal\feeds\Entity\FeedType->getMappingTargets() (line 305 of /var/www/clients/client1/web39/web/repos/ccmar/web/modules/contrib/feeds/src/Entity/FeedType.php)

    it looks like we must use standard profile installation because the fields are hardcoded.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States amanire

    I've updated the patch in #80 for Drupal 10 compatibility. The corresponding feed had stopped importing and was throwing the following warning:

    Entity queries must explicitly set whether the query should be access checked or not. See Drupal\Core\Entity\Query\QueryInterface::accessCheck().

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States amanire
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States amanire

    OK this patch now includes the explicit accessCheck for D10 compatibility.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States amanire
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States ed2908

    On Patches
    2928904-58.patch
    add_a_mapping_target_to_media_field-2928904-80.patch
    and
    add_a_mapping_target_to_media_field-2928904-86_0.patch

    I'm seeing a lot of:
    "The file, 917, failed to save because the extension, 17, is invalid.
    The file, 918, failed to save because the extension, 18, is invalid.
    The file, 920, failed to save because the extension, 20, is invalid."

    and a lot of "Image (field_media_image): This value should not be null."

    My mappings are:
    Feed field containing public://PATH TO IMAGE.jpeg -> Image (field_media_image)
    Feed field with file id -> Image (field_media_image): File ID
    Feed field with filename -> Name (name)

    My file IDs from the old site are 917, 918, 220 ect.
    Nowhere in the feed is 17, 18, 20. Certainly not being used as a file extension.

    Am I doing something completely wrong?

  • ๐Ÿ‡ง๐Ÿ‡ทBrazil isa.bel Balneรกrio Camboriรบ

    I had some issues after updating to the latest stable version of the module, but using the patch from #84 worked for me!

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States jesss

    Patch #86 is working for me!

  • ๐Ÿ‡ณ๐Ÿ‡ฑNetherlands djschoone Breda

    @jesss on which version did you apply? #86 on 3.0.0-beta4 gives me when visiting admin/structure/feeds/manage//mapping this error:

    Error: Call to a member function getSettings() on null in Drupal\feeds\Plugin\Type\Target\MediaTargetBase->__construct() (line 58 of modules/contrib/feeds/src/Plugin/Type/Target/MediaTargetBase.php).
    Drupal\feeds\Plugin\Type\Target\MediaTargetBase::create(Object, Array, 'media_image', Array) (Line: 21)
    Drupal\Core\Plugin\Factory\ContainerFactory->createInstance('media_image', Array) (Line: 22)
    Drupal\feeds\Plugin\Type\FeedsAnnotationFactory->createInstance('media_image', Array) (Line: 83)
    Drupal\Component\Plugin\PluginManagerBase->createInstance('media_image', Array) (Line: 515)
    Drupal\feeds\Entity\FeedType->getTargetPlugin(6) (Line: 375)
    Drupal\feeds\Form\MappingForm->buildRow(Array, Object, Array, 6) (Line: 160)
    Drupal\feeds\Form\MappingForm->buildForm(Array, Object, Object)
    call_user_func_array(Array, Array) (Line: 536)
    Drupal\Core\Form\FormBuilder->retrieveForm('feeds_mapping_form', Object) (Line: 283)
    Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 73)
    Drupal\Core\Controller\FormController->getContentResult(Object, Object)
    call_user_func_array(Array, Array) (Line: 123)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 627)
    Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
    Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 181)
    Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 76)
    Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
    Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
    Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 28)
    Drupal\Core\StackMiddleware\ContentLength->handle(Object, 1, 1) (Line: 106)
    Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
    Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 53)
    Asm89\Stack\Cors->handle(Object, 1, 1) (Line: 48)
    Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
    Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 36)
    Drupal\Core\StackMiddleware\AjaxPageState->handle(Object, 1, 1) (Line: 51)
    Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 704)
    Drupal\Core\DrupalKernel->handle(Object) (Line: 19)
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States jesss

    I'm on 3.0.0-beta4. Did you clear cache after applying the patch?

  • ๐Ÿ‡ณ๐Ÿ‡ฑNetherlands Nicasso

    I'm on 3.0.0-beta4 too and patch #86 does indeed generate an error when visiting: /admin/structure/feeds/manage/something/mapping

    A cache rebuild did not solve it unfortunately.

    Error: Call to a member function getSettings() on null in Drupal\feeds\Plugin\Type\Target\MediaTargetBase->__construct() (line 58 of modules/contrib/feeds/src/Plugin/Type/Target/MediaTargetBase.php).

  • First commit to issue fork.
  • Merge request !161#2928904 Added media target โ†’ (Open) created by jidrone
  • Open in Jenkins โ†’ Open on Drupal.org โ†’
    Core: 9.5.5 + Environment: PHP 7.4 & MySQL 5.7
    last update 8 months ago
    704 pass, 4 fail
  • Open in Jenkins โ†’ Open on Drupal.org โ†’
    Core: 10.2.x + Environment: PHP 8.2 & MySQL 8
    last update 8 months ago
    704 pass, 4 fail
  • Open in Jenkins โ†’ Open on Drupal.org โ†’
    Core: 9.5.5 + Environment: PHP 7.4 & MySQL 5.7
    last update 8 months ago
    716 pass
  • Pipeline finished with Success
    8 months ago
    Total: 1354s
    #134995
  • Open in Jenkins โ†’ Open on Drupal.org โ†’
    Core: 9.5.5 + Environment: PHP 7.4 & MySQL 5.7
    last update 8 months ago
    716 pass
  • Status changed to Needs review 8 months ago
  • Open in Jenkins โ†’ Open on Drupal.org โ†’
    Core: 9.5.5 + Environment: PHP 7.4 & MySQL 5.7
    last update 8 months ago
    708 pass, 4 fail
  • ๐Ÿ‡จ๐Ÿ‡ดColombia jidrone

    Hello,

    The existing patch has the following issues:

    • The media targets provided are tightly coupled to other existing targets.
    • It supports only image and file media types, so if you have a media type with a different name, it will not work.
    • It assumes the source file of the media entity types always follows the pattern field_media_[media_type_id].
    • The comments from the maintainer in #79 โœจ Add a mapping target to media field Needs review where not addressed.

    This new MR, has the following approach:

    • As minimal as possible changes to existing targets.
    • All the code is in just one new target called Media for loose coupling.
    • Maintainer comments addressed to ensure it is only applicable when media module is enabled
    • It is only applicable when the media field has at least one media type that contains a source field of type file.
    • Detects the applicable media type depending on the file extension.

    I know there are still improvements to do and it will need tests, but this could be a better starting point.

  • The last submitted patch, 86: add_a_mapping_target_to_media_field-2928904-86.patch, failed testing. View results โ†’
    - codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

  • Pipeline finished with Success
    8 months ago
    Total: 1368s
    #135039
  • ๐Ÿ‡ณ๐Ÿ‡ฑNetherlands megachriz

    Thanks @jidrone! That looks like a big step in getting this issue done. ๐Ÿ˜ƒ

    Who wants to make a start on writing the tests?

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States irinaz

    @MegaChriz, I am wondering if we could ask GitHub copilot to write tests. I want to experiment and see what AI can do these days. I can provide code of the module the module and file " src/Feeds/Target/Media.php" ask a question "please write unit test for xxxx function". I wonder what information AI needs to write suggested unit tests.

  • ๐Ÿ‡ณ๐Ÿ‡ฑNetherlands megachriz

    @irinaz Nice idea to ask AI for writing tests! Maybe they can help a little on the unit tests. But I think we do also need Kernel tests here, that can check if media gets imported as expected. I wonder if AI would be able to figure out how to write tests for that. It would need to know how a Feeds import goes programmatically, it needs to know what data to feed and it needs to know how the imported media should look like.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States irinaz

    @MegaChriz, thanks! We probably already have examples for other Targets, correct? We would add examples of data with media, probably. I am quite interested in testing what we need to provide to AI and what can be automated.

  • Pipeline finished with Failed
    7 months ago
    Total: 47s
    #145344
  • Pipeline finished with Canceled
    7 months ago
    Total: 993s
    #145346
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States irinaz

    Hi @megachirz and @jidrone,
    I did an experiment with writing kernel test using AI. I will try to run tests next week, but wanted to share some progress.

  • First commit to issue fork.
  • Pipeline finished with Failed
    about 1 month ago
    Total: 47s
    #317216
  • Pipeline finished with Success
    29 days ago
    Total: 289s
    #318344
  • ๐Ÿ‡ฎ๐Ÿ‡นItaly Giuseppe87

    I have added an extra settings to the current patch, that let the user choose to save the new media with the name of the entity or the file.

    This could be useful, as it's common to have multiple entities with the same files, and it isn't ideal to have the media name given by the first entity imported.

    However being an extra\different approach I haven't updated the MR.

  • ๐Ÿ‡ฎ๐Ÿ‡นItaly Giuseppe87

    Updated the patch of #104

  • Pipeline finished with Success
    28 days ago
    Total: 447s
    #319308
Production build 0.71.5 2024