Replies: 13 comments
-
Hello @dlinstedt IMO, this is way too case specific. Since WordPress attachments are in fact just another post type that can be attached to anything it's difficult to determine exactly when to remove an attachment. My view on this is to keep such a feature custom or in a separate addon. |
Beta Was this translation helpful? Give feedback.
-
Hi Jory,
If you noticed, the code is specific to the single post-id that is being
saved.
So it does not affect *any* other attachments, except the ones for that
specific post where the post was attached to the file previously.
Again, the code is highly focused - to limit any damage to other posts, and
ignore all other attachments that are there.
This basically just keeps "garbage" from being uploaded and left all over
the media folder.
Thanks,
Dan
…On Mon, May 25, 2020 at 9:14 AM Jory Hogeveen ***@***.***> wrote:
Hello @dlinstedt <https://github.com/dlinstedt>
IMO, this is way too case specific. Since WordPress attachments are in
fact just another post type that can be attached to anything it's difficult
to determine exactly when to remove an attachment.
My view on this is to keep such a feature custom or in a separate addon.
@sc0ttkclark <https://github.com/sc0ttkclark> ??
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5724 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABK3HVXF23T3R4BUG5PYNE3RTJVKNANCNFSM4NG3GM2A>
.
|
Beta Was this translation helpful? Give feedback.
-
I noticed that, but even then media attachments could have been used in more locations. It all depends on how you've created your site. I can imagine situations where this might be preferable but I can also imagine situations where this is not. I fully understand your point of view but I need to view this in a larger perspective. That is why I think an addon or a snippet would be best. The user then has to have a more in depth understanding of what such a change means. Otherwise I'm sure our support channels will get new topics of users that lost their media files while using such a feature but not fully understand it. |
Beta Was this translation helpful? Give feedback.
-
Agreed - and I can easily fix the SQL to accommodate / ignore attachments
that are part of more than one post, until they are "disassociated" with
all posts.
Even then, it would require an admin checkbox to "enable" the feature, and
documentation to explain it.
No worries, will keep it as a snippet for now.
…On Mon, May 25, 2020 at 11:31 AM Jory Hogeveen ***@***.***> wrote:
I noticed that, but even then media attachments could have been used in
more locations.
Even though a media attachment can only have one parent (the post it has
been uploaded to) that doesn't mean it doesn't have more parents. This is a
limitation of post types within WordPress.
I can use the same image in all my posts and still WordPress thinks only
one of them is the parent.
It all depends on how you've created your site. I can imagine situations
where this might be preferable but I can also imagine situations where this
is not.
I fully understand your point of view but I need to view this in a larger
perspective. That is why I think an addon or a snippet would be best. The
user then has to have a more in depth understanding of what such a change
means. Otherwise I'm sure our support channels will get new topics of users
that lost their media files while using such a feature but not fully
understand it.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5724 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABK3HVUWVDLNT2WWX265HVLRTKFNJANCNFSM4NG3GM2A>
.
|
Beta Was this translation helpful? Give feedback.
-
Unfortunately you can't. If you add an image in your content this is done hardcoded, there isn't any form of association made. Anyhow, could still be a nice addon option for some users. But I think such a feature should also make any attachments uploaded to such a field "exclusive" so they cannot be used in the regular media library. So in effect you'd have an internal library only available for that field. |
Beta Was this translation helpful? Give feedback.
-
Alright you win. However please note this:
1. Any file uploaded with PODS file attachment to begin with (to the pods
field) does receive (as you know) a POST attached by parent to the original
CPT
2. Therefore, any file (image or otherwise) previously managed by a PODS
CPT can actually be located using this method
3. Therefore, any file "still attached to the POST" but not found as
"active" in the JSON list (for current post or revisions) can be viewed as
disassociated and unused garbage.
UNLESS: PODS allows "attaching / associating" previously uploaded media -
as a "file attachment field" without ever creating a single sub-post
attachment type for it.
Then of course, all bets are off - and as you say, my thought process is
custom to purpose.
Anyhow, just thought I'd voice my opinion.
…On Mon, May 25, 2020 at 1:12 PM Jory Hogeveen ***@***.***> wrote:
Agreed - and I can easily fix the SQL to accommodate / ignore attachments
that are part of more than one post, until they are "disassociated" with
all posts.
Unfortunately you can't. If you add an image in your content this is done
hardcoded, there isn't any form of association made.
Anyhow, could still be a nice addon option for some users. But I think
such a feature should also make any attachments uploaded to such a field
"exclusive" so they cannot be used in the regular media library. So in
effect you'd have an internal library only available for that field.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5724 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABK3HVQ7DY3CBDFQP5Z354DRTKRHDANCNFSM4NG3GM2A>
.
|
Beta Was this translation helpful? Give feedback.
-
Hi @dlinstedt It's not about winning ;) please don't get me wrong, I too was previously thinking about such a feature but came to the conclusion that the current way WordPress handles attachment makes this difficult to be fool/error proof. Case 1:
Case 2:
Then I'm not even talking about other plugins possibly using the same attachment file link under different metakeys etc. etc. That is the reason I'm a bit careful with adding an option like this in Pods core until WordPress has a proper way of tracking unused attachments. |
Beta Was this translation helpful? Give feedback.
-
Understood :) Will consider these things, I was under the impression that
PODS meta file attach always uploaded new files, and didnt allow to choose
existing ones. I think there might be a way, just have to consider it.
For now, my snippet will work for my purposes.
Thanks
…On Mon, May 25, 2020 at 4:13 PM Jory Hogeveen ***@***.***> wrote:
Hi @dlinstedt <https://github.com/dlinstedt>
It's not about winning ;) please don't get me wrong, I too was previously
thinking about such a feature but came to the conclusion that the current
way WordPress handles attachment makes this difficult to be fool/error
proof.
Case 1:
- User uploads attachment in the library in another post.
- User then adds a new post and instead of uploading a new attachment
in the field, just selects a previous one from the library
- The link between the second post is never made. So If this option is
enabled the second post would lose it's attachment as well.
Case 2:
- A user adds a PDF file in a general post. Within the content of that
field. Thie is not JSON, just plain HTML through the WordPress editor.
- User then adds a new post and again instead of uploading a new
attachment in the field, just selects a previous one from the library
- If that second post removes it's attachment it will be deleted
resulting in a dead PDF file link in the previous (first) post.
Then I'm not even talking about other plugins possibly using the same
attachment file link under different metakeys etc. etc.
That is the reason I'm a bit careful with adding an option like this in
Pods core until WordPress has a proper way of tracking unused attachments.
I do believe there are some third party plugins that do that but they
encounter similar issues.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5724 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABK3HVVKP3BFBAM6HKM3AHTRTLGN7ANCNFSM4NG3GM2A>
.
|
Beta Was this translation helpful? Give feedback.
-
@JoryHogeveen Is this snippet actually usable just as it is or does one need to change something to work with my own CPTs? Like changing something to the respective slug? |
Beta Was this translation helpful? Give feedback.
-
Hi @secondsky |
Beta Was this translation helpful? Give feedback.
-
@JoryHogeveen I am aware. In my case media is only attached to one CPT only. |
Beta Was this translation helpful? Give feedback.
-
Hi @secondsky |
Beta Was this translation helpful? Give feedback.
-
@JoryHogeveen |
Beta Was this translation helpful? Give feedback.
-
Issue Overview
Enhancement Request
Subject: PODS file attachment management
Feature: During EDIT of a file attachment, when a file is "replaced" with a new upload / file, please delete the old attachment out of the uploads directory.
Expected Behavior
Desired result:
WHEN a file (any file) in single file or multi-file list is "replaced/edited"
THEN delete old file in MEDIA/UPLOADS
and DELETE old POSTS entry (this is critical to keep wp_posts clean too)
Optionally: Add a checkbox to the settings of the Custom Post Type in the PODS creator, add the checkbox to the FILE upload type:
[ x ] delete old files when edited/replaced
and when turned on, fire that action as described above.
OR sell this as an add-on
Current Behavior
Result: check Media Folder -> old attachment still lives on
Check wp_posts -> old attachment still has PARENT ID = POST 1 and still exists in POSTS
check wp_postmeta -> Post meta for POST 1 now points to new attachment (correct behavior)
Steps to Reproduce (for bugs)
Setup to see whats happening:
Possible Solution
Install SNIPPETS, then install the PHP action hook code below
WordPress Environment
wordpress 5.4.1
Elementor Pro 2.9.8
Astra Pro 2.5.0
PODS Pro 2.7.20
Pods Package Export (helpful!)
Workaround or Alternate Solution Until Bug is Addressed
BELOW IS SHORTCODE THAT ACTUALLY DOES WHAT IS DESIRED ABOVE
EXCEPT: IT DOES NOT ADD A CHECKBOX TO THE OPTIONS PANEL
Related Issues and/or PRs
Beta Was this translation helpful? Give feedback.
All reactions