Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wri 347 filename update #359

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Wri 347 filename update #359

wants to merge 7 commits into from

Conversation

komejo
Copy link
Contributor

@komejo komejo commented Mar 5, 2025

What issue(s) does this solve?

Adds transliteration, prevents useless filenames such as 01_24.jpg.

What is the new behavior?

Warns about bad filenames, switched out bad names with a transliterated name field value.

Profile requirements:

  • Does deploying this change require a change to config at the site level (choose one)?
    • No config change is required.
    • Yes, and I have written an update hook to apply these config changes.
    • Yes, and I've included updating instructions to be added to the release notes. The next release will need to be a major version increase. (Only do this in special cases.)

Site-level pull requests for testing. Only merge when these PRs are approved:

Create or update any site-level pull requests following the documentation

Checked on develop (TA to do)

  • Brasil
  • China
  • Indonesia

@komejo komejo requested a review from mariacha March 5, 2025 03:46
Copy link
Collaborator

@mariacha mariacha left a comment

Choose a reason for hiding this comment

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

This is generally working really well but I did find an bug on the node/add version of the media upload form.

I also appreciate the debugging messages but worry about them filling up the logs. Could you either comment them out or throw a flag around them so they only get logged if we've got debugging messages turned on? If you did a drush generate form:config in the wri_admin module with just one checkbox "Debug" it could do the trick.

@@ -0,0 +1,13 @@
description:
Copy link
Collaborator

Choose a reason for hiding this comment

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

This file exists already once this comes in as a module. Can you try moving it to the config/install folder directly in the root? I think that will fix the error circle is throwing.

If it works, you can update the path in the update hook too (but you can leave it in the wri_admin module)

@@ -99,3 +105,114 @@ function wri_admin_theme_suggestions_entity_moderation_form(array $variables) {

return $suggestions;
}

/**
* Validation function to ensure the media name is not just numbers.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Double comment here!

\Drupal::logger('wri_admin')->notice('Form state keys: @keys', ['@keys' => implode(', ', $form_state_keys)]);

// Check if the 'name' field exists in the form state.
$name_values = $form_state->getValue('name');
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like if you upload the file from a node page, the value is located within "Media" -- Try uploading a file named '1.jpg' as the hero image on an article.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants