Porta Westfalica
Account created on 9 May 2008, over 17 years ago
  • Drupal Software Engineer & Developer, Project Management at DROWL.deΒ 
#

Merge Requests

More

Recent comments

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

I created #3274873: Remove RSS publishing service from system.module β†’ in the past because from day 1 of modern Drupal I didn't really understand why RSS was still in core and not even in a separate (core) module or simply a views plugin. Especially for the RSS-Feeds menu item that's always there and confusing for site builders.

From my perspective, RSS should not be "hard-coded" into core, but should at least be a minimal optional module or receipt. And as RSS's nature is a list, views is the right place for it IMHO. I'd even vote for contrib, but if core, it should not be "just there" as a service and especially not as admin menu item you can't really get rid of (that's what I think is most confusing at all).

I understand that from history as a community and blogging platform, there were reasons to maybe add it directly into core. But so many other things have been thrown out of core in the past that might have been helpful for certain solutions, and they are well-placed in contrib...
Since views went into core, it should have just lived there.

Please do NOT see this as argument against RSS and its relevance for open source, decentralization and free speech. I just think that all this doesn't belong into the core of Drupal as framework, which should not make the assumption that this is useful e.g. for an eCommerce project or any other kind of Drupal project that needs no RSS.

Just my opinion. ;)

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

anybody β†’ created an issue.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

@jan kellermann I just came across this and think that maybe this is best practice to be added as install config directly in the klaro module?
https://git.drupalcode.org/project/klaro/-/tree/3.x/config/install?ref_t...

Or should it be a posthog submodule? What do you think?

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Thanks @grimreaper for the confirmation, so let's hope this can be merged soon. Our logs have been flooded for years with this now ...

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Thanks @steffenr all fine :)

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

anybody β†’ created an issue.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

For the records:

{% for order_item in order_entity.getItems %}
  {% set order_item_product = order_item.getPurchasedEntity %}
  <tr>
    <td>
      {{ order_item.getQuantity|number_format }} x
    </td>
    <td>
      {{ order_item_product.getSku }}
    </td>
    <td>
      <span>{{ order_item.label }}</span>
      <span style="float: right;">{{ order_item.getTotalPrice|commerce_price_format }}</span>
    </td>
  </tr>
{% endfor %}

The important part in the loop is:

{% set order_item_product = order_item.getPurchasedEntity %}
{{ order_item_product.getSku }}
πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Update: That was a mistake on my side, it's not possible!

So back to the initial request. Of course, we can provide a MR if you agree that's a valid use-case, even if an edge-case.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Thanks for the super quick reply @steffenr

It's not for testing. We're somtimes entering our key to translate entities for the client in production and then remove our key once finished. (Smaller clients). So in my personal opinion it's a valid use-case to be able to remove it again. But you're the maintainer! :)

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Thanks @avpaderno - we'll do so! So let's close this won't fix to save your time! :)

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Thanks @avpaderno! Yes I'll definitely do so!

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Thanks @avpaderno! Yes I'll definitely do so! I'll definitely be interested.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Sorry, just found out that despite the required mark on the field you can erase the API key and save.

So this is just a minor question on UX?

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

anybody β†’ created an issue.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Clear +1 on adding the ability to set the list size in the UI. By default it's just a few entries and there doesn't seem to be a way to change that in the UI?

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Thank you @marcoscano!!

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

@vasike I guess the maintainers here should create a new 2.x branch with Drupal 11.2 minimum compatibility? Or would you maybe like to apply as (Co-)maintainer? Do you know if they're still active?

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

GREAT WORK @vasike! Thank you!

Regarding layout paragraphs please see πŸ› Cloning Entities containing Layout Paragraphs breaks structure Needs work and let's maybe create a follow-up to add the missing parts over there!
Being able to clone (layout) paragraphs (with translations etc.) is super important indeed.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica
πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

As I think not many people are using LB 1.x any more, we should probably close this and focus on a clean solution in layout_paragraphs:

FYI: New hooks hook_entity_duplicate() and hook_ENTITY_TYPE_duplicate() have been committed to Drupal core and will be available in Drupal 11.2. A universal approach that works for all the cloning modules might be possible using those hooks, when sites are running 11.2+.

See #31 πŸ› Cloning Entities containing Layout Paragraphs breaks structure Needs work in πŸ› Cloning Entities containing Layout Paragraphs breaks structure Needs work

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

@guiu.rocafort.ferrer are you planning to further work on MR!69 in the meantime? I tried replicating it to 2.2.x and 3.0.x but then saw that it seems to be incomplete compared to #15? So definitely needs work, while

#31 seems to be the correct way to go.

Very much hoping for feedback from @justin2pin :)

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

anybody β†’ changed the visibility of the branch 3.0.x to hidden.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

anybody β†’ changed the visibility of the branch 3218226-cloning-entities-containing to active.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

anybody β†’ changed the visibility of the branch 3218226-cloning-entities-containing to hidden.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

anybody β†’ changed the visibility of the branch 2.2.x to hidden.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

anybody β†’ changed the visibility of the branch 3218226-cloning-entities-containing to active.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

anybody β†’ changed the visibility of the branch 3218226-2.x-cloning-hotfix-based-on-15 to hidden.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

anybody β†’ changed the visibility of the branch 2.0.x to hidden.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

anybody β†’ changed the visibility of the branch 3218226-cloning-entities-containing to hidden.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Great news @godotislate!!
@justin2pin what do you think regarding that and the state of this issue? There still seems no clean way to clone nodes with Layout Builder on it? How should we proceed?

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

@grevil: Could you finish the work here please? I removed the draft state. Maybe you could also try contacting the maintainer on Slack if the fix is fine, so we can finally get this committed?

@thomas.frobieter can tell from TT if this works as expected.

Latest rebase from MR!3 attached as static patch!

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

anybody β†’ created an issue.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica
πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica
πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica
πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

anybody β†’ created an issue.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

anybody β†’ changed the visibility of the branch 3553841-maybe-further-tests to hidden.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

I'll add another case in tour_dashboard_test very similar to our case!

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Whao, I think I solved it... Please take a look. Also works for our real-world case now.

Better review carefully to ensure I didn't make a mistake like the one before me ;)

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

What do you think about #5 looking at the code?

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica
πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Thanks @smustgrave - hope I got you right?

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Test-only branch fails as expected and exactly shows the issue we have.

I'll further work on the fix.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

tour.tour.tour-test.yml defines multiple routes, but they are all distinct:

routes:
  -
    route_name: tour_test.1
  -
    route_name: tour_test.1_no_action
  -
    route_name: tour_test.3
    route_params:
      locale: foo
  -
    route_name: node.add

Which seems impossible in my example. So maybe the case given in the issue summary has never been thought of and tested?

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

@smustgrave here we go. Tests are on the way. Reading through the code I also found #3553841: Routes evaluation is wrong if more than one route is given β†’

I guess Tour simply was never used with multiple routes for the same tour?

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

I added some lines of documentation to better explain what it does. That would have already helped in the past to better understand what it's meant for. I'm at least pretty sure that assigning an ID to the "bundle" key here is wrong, also comparing that to the other lines where it's done correctly.

The hard-coded $route_match->getParameter('node')) also looks a bit risky to me, for example for other entity cases, but that's a different story and I'm not totally sure about the consequences.

case 'bundle':
  case 'node_type':
    if (($entity = $route_match->getParameter('node')) && ($entity instanceof EntityInterface)) {
      if ($entity->bundle() === $tour_param) {
        $params['bundle'] = $entity->bundle();
      }
    }
    break;

Should maybe be checked in a separate issue.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica
πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica
πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

anybody β†’ created an issue.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Sorry @avpaderno I didn't know that. Will ping him, he's back on Monday.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Thank you all! I just came across this wonderful improvement πŸŽ‰and had to remember this still existing and slightly related Views Core "Reset" button bug with AJAX views: πŸ› On views with AJAX enabled, exposed filter "reset" causes page load Active
Maybe this can now be solved better based on this improvement? If anyone has a good idea for a resolution, that would wonderful.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica
πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Thanks @grevil! I agree this module is super helpful until this might one day (or maybe never...? ;)) be solved in core. As long as possible we should keep this as option for cases that match.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica
πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Moving over to Moving over to Drupal.org project ownership (2209259)

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Moving over to Drupal.org project ownership (2209259)

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

PS: Furthermore I think that many "base" functionalities might be similar or even the same, so would make sense to share that also on the UI level, e.g. forms. Shouldn't be much different to handle a block for newsletter subscription or a commerce checkout step with newsletter subscription checkbox for example one level deeper. So with a base module common functionality can be shared across all levels?

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

@stijnstroobants I'd think of a master "Brevo" module that allows storing the API credentials and provides the basic API methods, maybe not even any end-user-facing functionality. Submodules and contrib should then base on that.

We implemented something similar for the Posthog module, if you'd like to take a look.
Please note that I'm not the Brevo module owner or key-maintainer, just using Brevo and try to help with the module roadmap. Where should we discuss that further?

Another alternative would be to keep this in contrib, but use the Brevo module as base for API credentials. But I think it would also be helpful to have you as experienced Brevo developer!

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Does anyone remember an existing core issue for this?

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

This is a major issue, especially for SEO / Metatag that uses tokens heavily, as incorrect words are being created.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

This is not limited to [node:summary] it also happens with [node:body], so seems to be a general bug when transforming HTML to text only?

Is there already a broader known core issue for that?

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

anybody β†’ created an issue.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Just fund that this is a duplicate of πŸ› Implementation of 2901287 causes issues when multiple instances of a flag are rendered on page Active and the bug was introduced in πŸ› Accessibility : Focus lost when link is clicked Needs work

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Thanks @ivnish I can confirm this issue and just ran into this using the last stable release: πŸ› Flag Action Ajax Link "focus" command jumps to wrong flag, if there are multiple on the page Closed: duplicate (I'll close it as duplicate of πŸ› Implementation of 2901287 causes issues when multiple instances of a flag are rendered on page Active which I just found.

So this definitely should not be added, see my issue. I don't think the focus can work properly, as we never know which of the buttons were clicked.

Could you maybe tag a new stable release with the reverted code? Thank you!

I'd vote to close this won't fix or we need a quite complicated solution, that might not be worth it? Another alternative might be to solve the focus directly on the clicked element in JS?

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica
πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

anybody β†’ created an issue.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica
πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Latest state of MR39 attached until this is merged. Would be great to have this fixed.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Tests need to be fixed, but code looks good to me in general!

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Thank you very very much for the immediate reply @jedihe. That sounds good.

BTW wouldn't it make a lot of sense to apply as co-maintainer of this module? I see a lot of activity from your side here, while most of the maintainers seem to be inactive. I also thought about offering maintainership to push things forward here...

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Nice!! Thank you @fgm!

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Thank you @anruether! Nice, I created πŸ“Œ Join forces and merge this into the Brevo module? Active accordingly. What do you think?

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

anybody β†’ created an issue.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica
πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

@kinta: Nice! Could you rebase this against 3.0.x? Does it work fine for you?

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Needs a rebase

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Needs a rebase

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

3.0.x now. Anyone interested to finish this?

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Is this still an issue?

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

@jedihe any negative side effects you see? How would you suggest proceeding?

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Should be rerolled against 3.0.x?

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica
πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

9Y later

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Thanks for the addition and cleanup!

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Is this still an issue in 3.0.x-dev?

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

@jeni_dc is this still an issue with 3.0.x-dev? Could you create a MR then to fix this?

Maybe a duplicate of πŸ› Non-static methods called as static Needs work ?

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Please use MRs now instead of patches!

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica
πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

@josh.estep could you please turn this into a MR against 3.x?

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

If anyone is interested, please provide a MR to re-add this! Thanks!

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Indeed they are not helpful. Please reroll against 3.x @urvashi_vora or @nehakhadke

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

If anyone is interested, please provide a MR for a dfp_cookies submodule! The cookies module has some great examples! Should not be too hard!

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

@jedihe any plans to proceed here?

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

@VladimirAus so should this be closed / fixed`?

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Please turn this into a MR if it's still relevant for 3.x

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

If this is still relevant for anyone, please provide a MR to provide that integration / compatibility. Otherwise let's close this.

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Is this still relevant for 3.0.x?

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

Shouldn't this maybe better be solved in contrib or custom by using a hook? Does #3220030: Implement hook_dfp_tag_alter() β†’ help maybe?

πŸ‡©πŸ‡ͺGermany Anybody Porta Westfalica

@jedihe I think this might still be helpful. Would you turn it into a MR? Any other places where tokens might make sense?

Production build 0.71.5 2024