🇦🇷UTC-3
Account created on 11 May 2005, about 19 years ago
#

Merge Requests

More

Recent comments

🇦🇷Argentina hanoii 🇦🇷UTC-3

This is a longish issue, I tried following but different things were going on. I happened to work on a fix about url access and I wanted to share my work here.

My recommendation would be to maybe try to get the access bit first (which is easier) and then something more robust for the breadcrumbs in general?

In anyway, the two MR can co-exists and I will be using on a product of mind.

This is pretty much the same fix that the one #3319445-26: Edit link in breadcrumb region shows even when user has no permission , and they likely should be applied together.

🇦🇷Argentina hanoii 🇦🇷UTC-3

The rebased one was missing some twig changes, I happened to worked on fixing this before having found this issue :facepalm:

I a very similar thing and went ahead and push it, it makes it cleaner in the the twig and uses the proper drupal services.

🇦🇷Argentina hanoii 🇦🇷UTC-3

hanoii changed the visibility of the branch 3319445-edit-link-rebased to hidden.

🇦🇷Argentina hanoii 🇦🇷UTC-3

hanoii changed the visibility of the branch 3319445-edit-link-in to hidden.

🇦🇷Argentina hanoii 🇦🇷UTC-3

hanoii made their first commit to this issue’s fork.

🇦🇷Argentina hanoii 🇦🇷UTC-3

Thanks! I thought about that, but I figured that that the change is very minimal that it would make it a burden to have to maintain two separate branches for some time only for this change unless there's a strong opinion that 2.x would is EOL and all will happen in a new 3.x.

🇦🇷Argentina hanoii 🇦🇷UTC-3

While maybe not exactly the same, I believe that Support the new assets stream wrapper on Drupal 10.1.0+ Needs review should help with this on 10.1+, at least make it store it locally instead of s3fs which is what I would expect for dynamically generated assets.

🇦🇷Argentina hanoii 🇦🇷UTC-3

hanoii changed the visibility of the branch 3454273-order-select-shows to active.

🇦🇷Argentina hanoii 🇦🇷UTC-3

hanoii changed the visibility of the branch 3454273-order-select-shows to hidden.

🇦🇷Argentina hanoii 🇦🇷UTC-3

Thanks! For some reason I thought that hook was not needed anymore and the sort plugin was discovered on its own. I even searched for it, but something must I obviously missed something. Apologies, 2.0.1 is out!

🇦🇷Argentina hanoii 🇦🇷UTC-3

After coming back to this module a long time, and looked at this issue, I think is a bit too complicated for the module. Not codewise, as the code is simple enough, so thanks for that, but support-wise, as I can see this nor working as expected in many scenarios are views arg are not always so literal.

Besides, the views argument sometimes gets into the sql so there might be ways this can still be used in the sort argument. If we were to add this I would like to have tests or something with different kind of arguments to make sure we are doing things properly.

Finally, formattedText seems and odd use case for this kind of things.

🇦🇷Argentina hanoii 🇦🇷UTC-3

Although old, the answer would be yes. There are many ways of doing this, but one could be removing the dot and casting them as integer.

🇦🇷Argentina hanoii 🇦🇷UTC-3

Although old, I tried to reproduce it and it just worked, and this module is not doing much with the entered expression.

🇦🇷Argentina hanoii 🇦🇷UTC-3

Understood. Although I'd recommend if that's the case to maybe release a new version with only support for D10.

Because the fact that 4.3.1 installs but with a very old solarium library it's potentially dangerous, I actually found another project that I didn't upgrade under this same situation. Or maybe fix -dev on composer to match the actual latest so that dev is also not picked up by composer for 4.3.1?

🇦🇷Argentina hanoii 🇦🇷UTC-3

Fix on the previous re-roll as schema was already there.

🇦🇷Argentina hanoii 🇦🇷UTC-3

Re-roll against 2.x

🇦🇷Argentina hanoii 🇦🇷UTC-3

Will this still allow to install 4.3.1 on Drupal 9.5.11?

🇦🇷Argentina hanoii 🇦🇷UTC-3

6 years after the last comment I needed this feature, as I wanted to offload this request to a multisite on with not a huge amount of resources.

Overall I feel it's quite good, I added an install hook, a configuration option and small theme tweaks so that you can see if the data is cached as well as an option to refresh it. See MR !8

🇦🇷Argentina hanoii 🇦🇷UTC-3

6.x-dev is actually a lot older than 6.2.3

🇦🇷Argentina hanoii 🇦🇷UTC-3

Following the remaining steps of the process - owner haven't replied yet - summary updated.

https://www.drupal.org/project/notify_bar

🇦🇷Argentina hanoii 🇦🇷UTC-3

I think a permission would just work, it is config but you set it as part of your deploy, so a role that have a perm can be assigned/removed from the user as necessary.

I can't think of an issue with the permission.

I also wanted to provide another means of setting the read only lock (globally) and I wondered if using state is also a nice alternative, as that can be changed on the fly through drush or other means.

🇦🇷Argentina hanoii 🇦🇷UTC-3

I've just contacted https://www.drupal.org/u/dhanrajlakhara through his Contact Form:

Hello,

I've already got maintainer access on https://www.drupal.org/project/projectownership/issues/3235895#comment-1... and I just filled in a request to become an owner of the module on https://www.drupal.org/project/notify_bar/issues/3397888 💬 Offering to take ownership of Notify Bar Active .

I've also fixed most issues and released a new version for D10 and D9.

It looks like you are not active in Drupal anymore, and I would like to take the lead on the project and becoming the onwer feels safer for the long run.

I would advise you to provide feedback on this request on https://www.drupal.org/project/notify_bar/issues/3397888 💬 Offering to take ownership of Notify Bar Active .

Thanks,
Ariel.=

🇦🇷Argentina hanoii 🇦🇷UTC-3

Fixed in 2.x

🇦🇷Argentina hanoii 🇦🇷UTC-3

Fixed in 2.x

🇦🇷Argentina hanoii 🇦🇷UTC-3

I am picking up on the feature @nehajyoti mentioned (she worked in the agency I am working on where we use this module) and improving upon the module which needs quite a few things, which I am willing to maintain.

Am I just added as a maintainer or can I own the module to take the lead upon it?

Thanks!

🇦🇷Argentina hanoii 🇦🇷UTC-3

hanoii made their first commit to this issue’s fork.

🇦🇷Argentina hanoii 🇦🇷UTC-3

I fixed it but apparently I can't resolve the thread. @jwilson3 maybe you can?

🇦🇷Argentina hanoii 🇦🇷UTC-3

Thanks! I personally don't like casting as a way to sort issues, I rather make sure that the issues don't happen or add proper conditionals. Most of this have been fixed as part of d10 compatibility work which will soon be released.

🇦🇷Argentina hanoii 🇦🇷UTC-3

Hmm, this is a patch for drupal core, probably not where this issue belongs and very likely outdated. For now closing, feel free to add it to the drupal core issue queue.

🇦🇷Argentina hanoii 🇦🇷UTC-3

This has been fixed in the upcoming 3.x releases

🇦🇷Argentina hanoii 🇦🇷UTC-3

Thanks a lot! This was already fixed by me before checking on this patch, this is already committed to 2.x. Soon a 3.x for d10 will be released.

🇦🇷Argentina hanoii 🇦🇷UTC-3

@gisle, do you mean because I assigned this issue to me? I am sorry, I read the issue description and it says:

A prospective new maintainer can take this issue after the 14 day window has
elapsed and take the following steps to be added as a maintainer:

I understood by "take this issue" that I needed to assign it to mayself and then do the sub bullet points. If I understood it wrongly, I apologize.

🇦🇷Argentina hanoii 🇦🇷UTC-3

As part of my line of work this module is being used and we'd like the option to update it both internally and for the community.

Thanks!

🇦🇷Argentina hanoii 🇦🇷UTC-3

I am gonna close this one, it doesn't look as an issue, trying out on https://simplytest.me/configure?project=config_ignore_auto just works.

🇦🇷Argentina hanoii 🇦🇷UTC-3

Just added you as a maintainer! Welcome!

🇦🇷Argentina hanoii 🇦🇷UTC-3

Thanks for the bug report and the MR. Although it's looks like pretty safe, I am having trouble reproducing it.

drush eval "var_dump(\Drupal::config('config_ignore_auto.settings')->get('ignored_config_entities'));"
array(0) {
}

The configuration entries should really never be null or empty string as they are set by default through the initial config.

Also, whitelist_config_entities should never be null because core.extensions should never be auto-ignored.

I rather try to understand why this happened to you, it doesn't look like a bug on the module

🇦🇷Argentina hanoii 🇦🇷UTC-3

I started researching what's going on around this issue. I will likely use 'make_unused_managed_files_temporary' for now, but I was wondering if a module that (as opposed to most of the modules out there that tries to add some smarter capability) that adds a setting to each file field so that you can manually set which file file flags a file for deletion when the entity is removed.

🇦🇷Argentina hanoii 🇦🇷UTC-3

I applied #2671162-116: Also use text editor (CKEditor) for "summary" of a text field on 11.x and manually edit the editor.js file as the es6 file was removed.

Also, some of the test files modified on the patch there were removed in #3270438: Remove CKEditor 4 from core so leaving the other tests.

🇦🇷Argentina hanoii 🇦🇷UTC-3

This is a duplicate of 🐛 Drupal 10 proper once dependency Fixed . Almost the same patch.

🇦🇷Argentina hanoii 🇦🇷UTC-3

@tzt20 the latest 2.0.2 includes these fixes.

🇦🇷Argentina hanoii 🇦🇷UTC-3

ON #12, I first re-rolled #6 into 1.x, then tried it a bit, and I noticed it didn't really work very well, you seem to need to import it twice for the fields to work.

I then tried #10, it didn't really work, a simple text field out of a vanilla drupal import on a menu item failed to import once exported (then removed) and then re-imported. It gave a fatal error on a missing field.

I then tried #7 and it actually worked pretty good. I think this could be the better solution. And although I wouldn't mind to include all of the dependency injection and coding standard fixes, I agree with #10 that for reviewing purposes the changes are a lot. It wasn't at first, but I now agree those changes could go on a separate follow up issue.

MR!9 opened with this change.

🇦🇷Argentina hanoii 🇦🇷UTC-3

Hmm, not sure on those, on my tests. I normally see the same conditions on both, at least they should be as they are using the same conditions plugins, maybe there's some filtering. I wonder where those are coming form, probably context_entity_field ?

🇦🇷Argentina hanoii 🇦🇷UTC-3

I tried context:^5 and entity_route_context:^4 on a real world site on D10 and this doesn't happen. I wonder if it's the new version that did something better.

🇦🇷Argentina hanoii 🇦🇷UTC-3

I am pretty sure this is not an error in this module, but rather in how context and it's ecosystem handle the conditions. I can't look at this right now, but the same things happens with blocks.

If you do the same with blocks, the block will not display in the same situation as google tag. If you uninstall google tag, the same things happens with block. this is because entity_route_context adds a new context to a condition from the context module and it's saved with that.

There are a lot of things going on that are outside of this module:
- I would have expected for the configuration to have a dependency on the new module if a context of that module is used, but this is not happening.
- Also context default are just wrong. If you just enable context and save google tag (or a block) you will see visibilty/conditions being output in the config. These works because those conditions are empty, but still, there should be none.

This module doesn't filter out defaults, core does it so I think that context is failing to provide the instance default properly so that it is filtered out.

All of this, I think needs to be fixed somehow on contexts, and if not, core, but I would think it can be fixed in context.

🇦🇷Argentina hanoii 🇦🇷UTC-3

In your steps to reproduce at which point you save d
The google tag setting with a value ?

🇦🇷Argentina hanoii 🇦🇷UTC-3

The module looks a bit dated:

https://git.drupalcode.org/project/context_entity_field

I wonder if there's something odd with that condition, but if there is, a similar (if not the same) should happen with blocks visibility. I use that as a baseline.

🇦🇷Argentina hanoii 🇦🇷UTC-3

I was about to try the context modules but my current test site is D10 (which is where most of my google tag fixes has been going on). I will try testing with a D9 sometime later but maybe I can help you troubleshoot the issue.

🇦🇷Argentina hanoii 🇦🇷UTC-3

your entity_route_context is the default condition or you are editing something? what happens if you save a block config when you save the block with the same visibility settings?

🇦🇷Argentina hanoii 🇦🇷UTC-3

I think I fixed this, took quite a bit of understanding on what was going on.

I also added a status label to the entity list builder to note when the status is overridden (copied the approach from config split).

🇦🇷Argentina hanoii 🇦🇷UTC-3

Hmm, it probably won't, but if you can try that as well and outline steps to reproduce that'd be great.

🇦🇷Argentina hanoii 🇦🇷UTC-3

MR is ready. The culprit was really this:

But I also cleaned up some other bits of the code. I removed a trait, I figured it was making an already complicated code even more obscure and I wanted to leave things as close to BlockForm.php as possible as this will make it easier to keep up with any changes on core if there's any around this logic.

I also think (not 100% sure) that what I did and explained on #3345719-6: Google tag code is not present is not necessary (I quickly tried and it looks it's not) but as BlockForm.php is doing it, I'd leave it there just in case. I did note this on the comments on a @TODO.

@NicholasS if you can try this that'd be great.

🇦🇷Argentina hanoii 🇦🇷UTC-3

I am working on this, mostly fixed, MR soon.

🇦🇷Argentina hanoii 🇦🇷UTC-3

conditions: {} means no conditions, so the tag should be displayed always, is that not what you are experiencing.

🇦🇷Argentina hanoii 🇦🇷UTC-3

But if you are casting you are removing any potential benefit of the strict comparison

🇦🇷Argentina hanoii 🇦🇷UTC-3

If we are going to cast, isn't it better to remove the strict comparison instead?

🇦🇷Argentina hanoii 🇦🇷UTC-3

MR created

🇦🇷Argentina hanoii 🇦🇷UTC-3

I think there's a critical issue with this on 2.0.0

The issue should be easy to reproduce.

- Add a single google tag with no condition
- Add a node without any taxonomy (or look at a page that's not a node)

And the google tag is not added.

I spent quite a bit of time trying to figure this out. The reason is that the empty conditions are stored in the config entity, thus requiring the element in the context (i.e. a vocabulary is expected to be gathered from the route, which is not possible if the node doesn't have any term).

I then started to look why this was happening and comparing with other visibility logic like block (which I assume is where most of the code/logic has been taken from) and found that one thing was not being done:

https://git.drupalcode.org/project/drupal/-/blob/af1eb719451a296bd2d7f1d...

When you add this, the following happens and I tried to understand why it happens. The reason is that negate is 0 (numeric 0) which is then strictly compared to false on:

https://git.drupalcode.org/project/drupal/-/blob/af1eb719451a296bd2d7f1d...

It fails to remove it.

However, it seems that by saving it twice (one on the submit handler and they by the entityform save method()

https://git.drupalcode.org/project/google_tag/-/blob/af8ad2037b4865f5c20...

The first time it's stored, the config is updated internally to match store schema (0 becomes false), so the second time the config entity is saved, the previous code works and the conditions is stored as empty (if no conditions obviously) so now the context is no longer required.

Basically, with no conditions the tag should be displayed everywhere.

🇦🇷Argentina hanoii 🇦🇷UTC-3

hanoii created an issue.

🇦🇷Argentina hanoii 🇦🇷UTC-3

Thanks for the feedback! There's really is not an API change, as everything continues to work exactly the same unless you set a setting.

🇦🇷Argentina hanoii 🇦🇷UTC-3

While this might still need work, I would like an initial review.

🇦🇷Argentina hanoii 🇦🇷UTC-3

Not supported any more

Production build 0.69.0 2024