This is a followup of #3059950: Media sources should handle empty source field values more gracefully. After a long discussion, we decided we have to accept the reality that media items can have an empty source field value. They normally shouldn't, but it happens in some cases eg.:
In #3059950: Media sources should handle empty source field values more gracefully we improved the handling of the NULL value so we don't throw a fatal error anymore. It would be nice to add logging for media items that have this problem, so editors can easily find those media items and correct the source field value if needed.
To be determined..
Discuss if logging should be added for NULL source field values, and if we want it, how to add it.
Write patch
Review
Commit
None.
None.
None.
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | add_logging_for_empty_source.patch | 710 bytes | karma86 |
Comments
Comment #3
chrisdarke commentedComment #5
ajv009 commentedYUp, I think we should throw a warning in watchdog, what say?
patch should be as simple as adding a logger where it identifies that its NULL gracefully :)
Comment #6
anjali rathodComment #7
anjali rathodShould we create an event subscriber in core image module for this feature?
Comment #8
ajv009 commentedNope, that will be too much for just reporting...
Instead, you can just add a drupal watchdog logger that will report a warning somewhere here https://www.drupal.org/project/drupal/issues/3059950
Comment #11
cindytwilliams commentedTagging this issue for first time contributors at DrupalCon Prague 2022.
Comment #12
rodrigoaguileraDoing some triaging before the Drupalcon Prague 2022 sprints for mentoring.
Since this issue still needs discussion I believe it is not a good candidate for novices.
Also un-assigning because we don't usually use the field for core issues.
This is a change that should be aimed at 10.1.x if it adds the logging.
@seanB can you name the people that are relevant for this discussion?
My opinion:
Yes there should a new log entry when an editor saves/edits a media item with an empty source.
Comment #13
karma86 commentedHello everyone!
I was looking into this issue and created a patch with a solution.
I'd love some feedback, I'm a novice regarding contributions.
Thanks!
Comment #14
marcoscanoI wasn't active in the issue that originated this one, so I might be missing some context. Here are a few things to consider though:
- In use-case example 1 in the issue description (site-builders make the media source field non-required), I feel it is unlikely that there will be someone checking the logs to detect this, so in practice I don't see the logs being useful for this site.
- In use-case example 2 (files being deleted), the log being done during the save operation wouldn't really add much value, since if the editor is saving a media item that had the file deleted, they've hopefully selected a new file? I could see a form error message / warning being more useful here.
- When performing migrations, it might be (temporarily) expected to have media items without the source field filled up, depending on how you've configured the migration. If we include a log to watchdog for every media item being saved, this could cause more harm than good filling up the logs with noise.
To me, when a media item has no source value, regardless of the reason, it's "broken". However I would argue that in normal circumstances it doesn't get into this state after a save operation, so maybe a log message there might not be the best approach? Things that come to mind:
- A way to mark these media items as broken somehow? (A flag in views? Hijacking the media label if the user is admin? Etc.)
- A status report message indicating if the site has "broken" media items. This implies querying all media content, which could get out-of-hand in sites with lots of entities.
I acknowledge none of these are ideal or feasible, so I'm mentioning them more in the spirit of brainstorming than actually suggesting them as implementation options... :)