Hmm, the only place I see in the code where an account is switched is FeedsExecutable->processItem()
bkosborne → created an issue.
This works as expected and restores the original toolbar. RE: #32, I think it's best that be handled in a follow up so we can restore this critical functionality.
Pencil icon was still broken in the latest MR. I fixed. Will merge and cut a 3.1.0 release soon.
bkosborne → made their first commit to this issue’s fork.
Thank you!
A site's UUID wouldn't be ideal here because we also need to ensure the hash tags are unique per a site's environments. E.g., if the site is synced from prod to a test environment, it will have the same UUID but shouldn't share the same cache tag "namespace" with production.
Using hostname is a safer choice, but still not perfect. I imagine people are using Drush for Purge invalidations, likely via cron. Some people may not be setting the hostname when invoking Drush, so it will end up being "default" or something, so the tags wouldn't be invalidated correctly.
Not sure what other option there is though. Perhaps make the cache tag prefix configurable and let the site owner decide?
Hi erickbj, I wonder if you're using this code in production since you've introduced it? Has it worked out well for you?
This is closely related to ✨ Add optional Environment setting Needs review . In fact, the solution here should be bundled with that change IMO.
Yes please. We don't use much from the SDK, should be straight forward to decouple from it. I'll get started on this now. I also agree that just copying code from the SDK isn't wise. We can get even simpler.
bkosborne → made their first commit to this issue’s fork.
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.
greggles → credited bkosborne → .
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.