-
Notifications
You must be signed in to change notification settings - Fork 571
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
Store 'file' ID only, not URL #1488
Comments
One quick off the top of my head benefit is not having to look up and fetch a URL, it's just already stored there. I think this topic has come up in the past and there was decision based on backwards compatibility potential issues, but it's been awhile and my memory may not be complete. |
Thanks for the reply, Michael. |
Another downside to saving the URL is that I've many times ended up forgetting to actually use the attachment rather than the direct value of the meta. Then once I'm using the code in a different environment (say, moving from development or staging to the production version), the URL doesn't work. Not a huge deal, but removing the URL would prevent that kind of mistake from happening in the first place. |
Can (Both the field name and I see the linked examples (eg https://github.com/CMB2/CMB2-Snippet-Library/blob/master/filters-and-actions/override-cmb2-data-source.php#L73-L98), though I'm not sure if that's what it's for, or how it would be done... eg. If you had a function hooked to |
I tried this pair. Any comments, please? Savingadd_filter('cmb2_override_logo_meta_save', 'override_meta_save', 10, 4);
function override_meta_save($override, $args, $field_args, $field)
{
$term_id = $args['id'];
$logo_id = $field->data_to_save['logo_id'];
$updated = update_term_meta($term_id, $args['field_id'], $logo_id);
return !!$updated;
} ... which successfully saves a new value as Retrievingadd_filter('cmb2_override_logo_meta_value', 'override_meta_value', 10, 4);
function override_meta_value($data, $object_id, $args, $field)
{
// Get term meta field 'logo
$logo_id = get_term_meta($object_id, $args['field_id'], true);
// Get attachment thumbnail object
$logo = wp_get_attachment_image_src($logo_id, 'thumbnail');
// Return URL of image to show
return $logo[0];
} On the term edit page, this successfully fetches and shows the 150x150 thumbnail of the ID which is set in |
If I recall those override filters, returning non |
SavingYeah... I can see how using the override succeeds in stopping the URL field getting set... though I can't quite figure out how to set the now-singular value as " I might have to just convert all my existing " RetrievingOn the display side, I think a |
The code workarounds I use above to save only an attachment ID to the meta, not an array also comprising the URL, hits a bump when it comes to file_list (multiple files). I'm inheriting As-is, the That's different from the filter workaround I hit on for a single I think it's going to require writing a similar pair of filter functions, though I'm not sure what. |
Nothing from me, I haven't attempted to think your usecase through enough to have an approach. |
Is your feature request related to a problem? Please describe.
No specific problem, but I don't understand why it's necessary to store the file URL when local attachments are the focus.
It's not a guarantee that the size variant given by the URL is the right one to serve.
The file URL is always resolvable by looking up the attachment by ID.
This allows for serving of the appropriate size etc.
Describe the solution you'd like
Docs say: "This field type will also store the attachment ID (useful for getting different image sizes."
But suppress storage of the file URL / explain why storing this is beneficial.
Describe alternatives you've considered
Any hook/action that would allow suppression of the URL save.
The
'url' => false
option seems to pertain only to the frontend (admin) visibility of the URL, rather than actually making URL storage false.Additional context
I'm aiming to move from ACF to CMB2 because it stored tens of thousands of needless rows (a meta item's pointer to the field descriptor). I'm trying to minimise database overhead.
The text was updated successfully, but these errors were encountered: