Just a note that if/when we implement this, we'll need a release note to indicate that the entire Cloudflare cache should be purged, because all the cache tags will have changed.
Patch from #8 appeared to have unrelated changes. I just updated the merge request to be compatible with latest from 7.x and hid the patch files.
Please, use MR updates for future updates, not patch files.
Nevermind, the latest patch breaks things. The linkit autocomplete isn't used at all with it. Needs some work.
looks good to me!
Okay, I refactored the logic here to be more precise in finding the correct title field to manipulate and to fix things so that if a title field cannot be found, the basic functionality of the selection handler (populating the uri input field with the clicked autocomplete value) still works.
Note that the logic for assigning $context variable is flawed as it doesn't scope correctly when there's no fieldset wrapping the link field (there won't be if the link field doesn't have a title field output).
const $context = $(event.target).closest('form,fieldset,tr');
But as far as I can tell, it's not that important and affects only people using LinkIt with CKE4. Given that's deprecated I think it's best not to touch it.
I marked 🐛 Linkit field doesn't work in nested Paragraph when Auto populate link text is enabled Active as a duplicate of this.
Maintainers, please add users "mahde" and "deulenko" to issue credits.
I'm confused about the patch in #4 as it is completely different from the patch in #3.
It sounds like the root of this problem is that the logic for finding the title field is flawed. There's other issues with this logic too that I discovered in 🐛 Field widget broken if there's multiple link fields, one without link text enabled Active . I created an MR in there to cover those issues as well as the issue here. I think this can be closed as a duplicate of that (I realize this issue was open first, but I'm not sure what's going on with the MR/patch here, and my issue covers additional issues. I'll carry over the issue credits to that one).
bkosborne → created an issue. See original summary → .
Note the LinkIt module provides a field widget for Link fields that does all of this and more. I can imagine LinkIt being absorbed into the core in the future.
This is old and seems unrelated to the module.
Agreed, looks good!
Agreed with this change, looks good.
Thanks. I'll commit this and release later this week.
Thanks! This is good to go. I did push another commit that explains why we're doing this too.
I put some more thought into this given your suggestion. But I've become more convinced that the current behavior is a bug and shouldn't be a choice someone can opt into. I think it's totally unexpected behavior to have a change to a node not update the most recent version of that node, but instead update the published version.
What I'd really prefer to do here is to fix this to load the latest revision, and wait and see if anyone has a need for the existing behavior. This module isn't widely used (yet) so I suspect there aren't any or many people using it with Content Moderation. Basically, I think it's a good idea to keep the module and config options as slim as possible while we can.
Ready for review.
bkosborne → created an issue.
Thanks!
Yes, adding Tugboat previews would be great. We use tugboat quite at bit at my work. If you create the issue I can work on it.
Ready for review.
Since the autocomplete widget and the multi-value select widget submit form values differently, I had to update our form submit handler to normalize the results, but I think it resulted in more readable code anyway.
Ah, nice find. Merging!
bkosborne → made their first commit to this issue’s fork.
Thanks!
eh, while I could do the change to select widget in a form alter, it's more complex that I'd like since we need to also load the term tree. Creating follow up to add it to the module...
bkosborne → created an issue.
Ready for review. Sorry, the changes look bigger than they are because I had to refactor how the form is constructed a bit. Now the code first gathers the list of all eligible term reference fields, then in a separate loop builds the form. This allowed me to collect the list of content types used by a field with the same name.
Agreed. I'll split that out into a separate issue. I may not even do it here. May be pretty easy with a form alter for our own use case.
bkosborne → created an issue.
Moving this issue to Media Entity File Redirect. I think that's the best place to solve this.
We came across this as well.
If the Media Entity File Redirect module (which I made) just modified the canonical path of media entities to be /download/[id], everything would just work. The reason the module doesn't do that is that the redirect route is optionally enabled per media type, and I don't think we can override the canonical link template per-bundle. It's the whole entity type. So instead, the module simply provides a route for the redirect path and provides LinkIt matcher and substitution plugins.
One possibility is to have LinkIt detect if Media Entity File Redirect is enabled, and if so, check if it's enabled on the entities type. This seems messy to me. I think it's best to avoid having one contrib module have feature detection of other contrib modules.
Another possibility is to fire an alter hook for the default value in the field widget? Media Entity File Redirect could then implement the hook and set the correct default value.
Without such a hook, the module would need to alter the field widget via media_entity_file_redirect_field_widget_single_element_linkit_form_alter, which may work. I'll investigate that.
Thank you, but we really need steps to reproduce here so I can be sure it's not some conflict with some other module.
I discussed with partyka and we think this is actually not a problem and the issue can be closed. At the very least, this should have more clearly defined description of what the problem is.
It sounds like what really is the problem is that the act of deleting and re-importing nodes created a ton of stuff in the trash with no clear way to purge the trash. Trash should have a purge all functionality or a multi-select for purging as was suggested in the issue summary.
But the current behavior I think makes sense, as I outlined in #6. An alternative behavior would be if Feeds didn't re-import items that were already trashed, but I think that's even more confusing.
Hi Rajab,
I'm still a bit confused about the distinction.
What makes you say that Bookmarks are visually marked anchors on the final front end? The documentation for the CKEditor plugin doesn't mention that.
The plugin implements the bookmarks in the same way that Anchor Link does. It creates a <a id="name"></a> element. That wouldn't be rendered on the front-end in either plugin unless you added specific styling for it.
Damn, my memory is failing me. I created that issue 8 days ago!?!
I thought this to, but the action plugin this module provides is currently applicable to nodes, which all have revision support.
So I think we can close this, or perhaps we change this issue to make the module work with other entity types? Though, at the moment I have no need for that, and prefer to keep the module as simple an narrow in scope as possible. There is Views Bulk Edit → that works with all entity types and has many more features that people could use as well.
bkosborne → created an issue.
Confirmed, it works well with layout builder. Thank you!
Ready for review!
Thank you!
Agreed we should be consistent! I converted all the local vars to snake case. I do use camelCase for everything most of the time out of habit, but I think the module already was using snake_case, so I switched to using that.
Thanks for the quick reviews!
Okay, ready for review!
I'm pretty sure if no version is present then Drupal calculates a hash of the file contents and uses that as the version on cache clear. Are you sure that's not happening? I know it works that way for asset aggregates.
If it works as I think it does, then I'd rather leave it as is. We're likely to forget incrementing that version on future updates.
Fixed! Once this is merged, I can start work on the other two issues I created.
Thanks for the review! I addressed the feedback and pushed.
Yea, I think we're good at just requiring Drupal 10, which requires PHP 8.1 already. I don't think I used any language features beyond 8.1.
Okay, MR added and ready for review.
Maybe! It may be a bit tricky to pull the logic out of the main module.
bkosborne → created an issue.
bkosborne → created an issue.
bkosborne → created an issue.
bkosborne → created an issue. See original summary → .
Duplicate of 📌 Require Views Bulk Operations in composer.json Active (I know this one was created first, but more activity in the other)
I cannot reproduce either. This issue is 3 years old so likely no longer relevant.
Not enough information here. Since the issue is 3 years old, going to close it out.
The module's README has this:
(Optional) Define a "bulk_edit" form mode for the entity bundles
that you wish to be able to bulk edit. Under the "Form modes" page
at /admin/structure/display-modes/form you can add a new form
mode. If it uses the machine name "bulk_edit", the form for the
"Modify field values" action will use that form mode to determine
what fields are available for bulk editing.
I spent a few days debugging the issue and determined that the Trash module was causing mayhem with Feeds.
What mayhem though? I think this needs more specific steps to reproduce. What exactly was the issue? (I know this is a couple months old and may not be fresh on your mind anymore!)
Here's what I did:
- Tested on a site with Feeds and Trash enabled
- Created a feed type to import from an RSS feed on some other website to a content type that had Trash enabled
- Ran the importer which imported a bunch of nodes
- Deleted one of the nodes (moved it to trash)
- Re-ran the importer
- Observed that the importer imported the trashed node again
This is what I expected would happen. When feeds went to check if the node already existed or not, it found that it did NOT exist because it was in the trash and the Trash module is "hiding" it from anything that goes looking for it.
I tested this again with standard profile on 10.4, 11.1, and 11.3 and I can no longer produce it. The first hit to a 404 path, X-Drupal-Dynamic-Cache gets UNCACHEABLE (poor cacheability), but if I make a 2nd request to the same 404 path or some other 404 path, I now get `HIT`.
This still confuses me, but it's better behavior than what I saw before (or thought I saw before).
Note I was testing just with standard profile, but I uninstalled Internal Page Cache so it didn't interfere with anything.
I'm tempted to close this, but catch did say there's some legit reason for some of this.
Sure, this was the fix for this module.
Yea this was missed.
We have only one feature that uses this, which is the client-side authentication method for gateway authentication. I imagine most sites don't use this, so we should consider not installing the module for everyone.
Updated the proposed resolution...
bkosborne → made their first commit to this issue’s fork.
Created a new branch from 3.x.
bkosborne → changed the visibility of the branch 3275551-when-bulk-adding to hidden.
bkosborne → changed the visibility of the branch 3.x to hidden.
bkosborne → changed the visibility of the branch 2.x to hidden.
bkosborne → created an issue.
Okay, trash module released 3.0.17 which includes my fix. So I think our tests here should pass now. I'll re-run them.
This would only apply to 4xx responses, because 5xx responses are already excluded from code above, which checks first if the original response is considered cacheable. It won't be with a 5xx code.
I'll take a look at adding this option this week...
Content editor wants to do something/anything with the original /policies page. When the content editor goes to /policies to **edit** that page, they are now redirected and probably quite confused. Even if the content editor knows what's going on, they would now have to find the node in the content listing /admin/content and click "Edit" to be able to edit the content.
This happens to our editors frequently. We don't use a patch from this issue, but our editors and team know that you can do the workaround of creating a redirect from an existing path by using the system path as the source. E.g., instead of creating the redirect from /policies -> new page, you create it from /node/123 -> new page. The end result is the same level of confusion. The only way to edit the original page is to find it in the content dashboard.
bkosborne → created an issue.
bkosborne → created an issue.
+1 to this!
Is it possible there's some other module interfering? This module's code is designed to not do anything if the "override" checkbox is not checked.
I'm not able to reproduce this. Do you have specific steps to reproduce on a clean install?
bkosborne → created an issue.
Agreed. I fixed it in the MR for that other issue, so I'll mark this as a duplicate.
Okay, the blocker from Trash is in. Just need to wait for its next release, then these tests should pas.