Skip to content

feat (media-library): emscli media library update file links#1572

Merged
Davidmattei merged 32 commits intoems-project:7.xfrom
YanisGroffier:feat/emscli-media-library-update-file-links
Mar 25, 2026
Merged

feat (media-library): emscli media library update file links#1572
Davidmattei merged 32 commits intoems-project:7.xfrom
YanisGroffier:feat/emscli-media-library-update-file-links

Conversation

@YanisGroffier
Copy link
Copy Markdown
Contributor

Q A
Bug fix? n
New feature? y
BC breaks? n
Deprecations? n
Fixed tickets? n
Documentation? n

Command to convert ems file links (in wysiwyg fields) into ems object links (if the file already exists in a media library)

@theus77 theus77 changed the title Feat/emscli media library update file links feat (media-library): emscli media library update file links Feb 6, 2026
Comment on lines +14 to +15
use RectorPrefix202601\Symfony\Component\Console\Input\InputArgument;
use RectorPrefix202601\Symfony\Component\Console\Input\InputOption;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not the right classes

Comment on lines +59 to +76
$this->options = new MediaLibrarySyncOptions(
//TODO If I have to put all args in this method, do I have to declare them before (in the object attributes) or can I simply put a bunch of nulls and false?
folder: $this->getArgumentString(self::ARGUMENT_FOLDER),
contentType: $this->getOptionString(self::OPTION_CONTENT_TYPE),
folderField: $this->getOptionString(self::OPTION_FOLDER_FIELD),
pathField: null,
fileField: null,
metaDataFile: null,
locateRowExpression: null,
targetFolder: null,
dryRun: false,
onlyMissingFile: false,
onlyMetadataFile: false,
hashFolder: false,
hashMetaDataFile: false,
);

}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you want to have a MediaLibrarySyncOptions? With all those options? I would recommand to first starts with simple class's members

foreach ($coreApi->search()->scroll($search) as $hit)
{

foreach($this->fields as $field) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not hesitate to create functions in order to avoid too much nested loops

Comment thread elasticms-cli/src/Command/MediaLibrary/UpdateFileLinks.php Outdated
Comment thread elasticms-cli/src/Command/MediaLibrary/UpdateFileLinks.php Outdated
Comment thread elasticms-cli/src/Command/MediaLibrary/UpdateFileLinks.php Outdated
Comment on lines +82 to +83
$this->io->section('Found Media Library files');
dump($this->logReports);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See in the colibri script how an excel is generated and uploaded in the admin

Comment on lines +113 to +115
if($match['content_type'] === 'media_file') {
$emsLink = EMSLink::fromMatch($match);
$this->logMediaLibraryLink($key, $emsLink, $value);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to log already "good" links

Comment thread elasticms-cli/src/Command/MediaLibrary/UpdateFileLinks.php Outdated
Comment thread elasticms-cli/src/Command/MediaLibrary/UpdateFileLinks.php Outdated
Comment thread elasticms-cli/src/Command/MediaLibrary/UpdateFileLinks.php Outdated
], $tempFile->path);
$filename = \sprintf('UpdateFileLinks - Rapport %s.xlsx', \date('YmdHis'));
$hash = $this->coreApi->file()->uploadFile($tempFile->path, 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', $filename);
$this->io->success($this->buildUrl($hash, 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', $filename));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the mime type not already define somewhere ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No but it can be put in a const to avoid repetition

Comment thread elasticms-cli/src/Command/MediaLibrary/UpdateFileLinks.php Outdated
@YanisGroffier YanisGroffier marked this pull request as ready for review March 9, 2026 12:06
@YanisGroffier YanisGroffier requested a review from a team as a code owner March 9, 2026 12:06
@YanisGroffier YanisGroffier requested a review from theus77 March 9, 2026 15:46
Comment thread elasticms-cli/src/Command/MediaLibrary/UpdateFileLinks.php Outdated
Comment thread elasticms-cli/src/Command/MediaLibrary/UpdateFileLinks.php Outdated
Comment thread elasticms-cli/src/Command/MediaLibrary/UpdateFileLinks.php Outdated
$this->mediaLibraryContentType = $this->getOptionString(self::OPTION_MEDIA_LIBRARY_CONTENT_TYPE);
$this->fileField = $this->getOptionString(self::OPTION_FILE_FIELD);
$this->coreApi = $this->adminHelper->getCoreApi();
$this->mimeType = $this->getOptionString(self::OPTION_MIME_TYPE);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really useful to me. The mimetype 'Type of spreadsheet document', 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet' should be defined in the class EMS\Helpers\Html\MimeTypes

$defaultAlias = $this->coreApi->meta()->getDefaultContentTypeEnvironmentAlias($this->contentTypeName);
$search = new Search([$defaultAlias]);
$search->setContentTypes([$this->contentTypeName]);
$search->setSources(['*']);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use $this->fields instead of retriving everything

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think about converting property accessor field to a elasticsearch field

Comment thread elasticms-cli/src/Command/MediaLibrary/UpdateFileLinks.php Outdated
Comment thread elasticms-cli/src/Command/MediaLibrary/UpdateFileLinks.php Outdated
Comment thread elasticms-cli/src/Command/MediaLibrary/UpdateFileLinks.php
if ($found && $this->force) {
$value = \str_replace($match[0], $found->jsonSerialize(), $value);
} else {
$this->logAssetLink($key, EMSLink::fromMatch($match), $value);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you log? not found or not $this->force


private function findMediaFileByHash(EMSLink $link, string $hash, mixed $value): ?EMSLink
{
$alias = $this->coreApi->meta()->getDefaultContentTypeEnvironmentAlias($this->mediaLibraryContentType);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be done once

theus77 and others added 2 commits March 16, 2026 13:36
 The following changes were applied :
 - Rename several variables for better clarity
 - Improve log handling when replacing asset links
 - Narrow the search scope of `setSources`
 - Move `mediaLibraryContentTypeEnvironmentAlias` declaration to initialization
@YanisGroffier YanisGroffier force-pushed the feat/emscli-media-library-update-file-links branch from 35f5a9b to 80a6818 Compare March 23, 2026 09:53
@YanisGroffier YanisGroffier requested a review from theus77 March 23, 2026 10:57
Comment thread elasticms-cli/src/Command/MediaLibrary/UpdateFileLinks.php Outdated
Comment on lines +177 to +183
if ($found) {
$status = 'Asset link found but not replaced.';
if ($this->force) {
$value = \str_replace($match[0], $found->jsonSerialize(), $value);
$status = 'Existing asset link successfully replaced.';
}
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if ($found) {
$status = 'Asset link found but not replaced.';
if ($this->force) {
$value = \str_replace($match[0], $found->jsonSerialize(), $value);
$status = 'Existing asset link successfully replaced.';
}
}
if ($found && $this->force) {
$value = \str_replace($match[0], $found->jsonSerialize(), $value);
$status = 'Existing asset link successfully replaced.';
} elseif ($found) {
$status = 'Asset link found but not replaced.';
}

$link = EMSLink::fromMatch($match);
$hash = $link->getOuuid();
$found = $this->findMediaFileByHash($link, $hash, $value);
$status = 'No Media Library object link found.';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add the file name or the hash in the message

$found = $this->findMediaFileByHash($link, $hash, $value);
$status = 'No Media Library object link found.';
if ($found) {
$status = 'Asset link found but not replaced.';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$status = 'Asset link found but not replaced.';
$status = \sprintf('Asset link found but not replaced for %s.', $emsLink->getOuuid());

@YanisGroffier YanisGroffier requested a review from theus77 March 24, 2026 11:10
@theus77
Copy link
Copy Markdown
Member

theus77 commented Mar 24, 2026

@Davidmattei Can you review and merge in this PR?

@Davidmattei
Copy link
Copy Markdown
Member

@Davidmattei Can you review and merge in this PR?

7.1 or 7.x ? The plan is to retag 7.1 somewhere next week

Comment thread elasticms-cli/src/Commands.php Outdated
@theus77
Copy link
Copy Markdown
Member

theus77 commented Mar 24, 2026

@Davidmattei Can you review and merge in this PR?

7.1 or 7.x ? The plan is to retag 7.1 somewhere next week

7.x is ok

@Davidmattei Davidmattei merged commit 954ff16 into ems-project:7.x Mar 25, 2026
1 check passed
@github-project-automation github-project-automation bot moved this from Todo to Done in Elasticms Mar 25, 2026
@Davidmattei Davidmattei added this to the 7.2.0 milestone Mar 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants