Valladolid
Account created on 24 April 2008, about 17 years ago
  • Senior PHP / Drupal Developer at Lullabot 
#

Merge Requests

More

Recent comments

🇪🇸Spain plopesc Valladolid

Documentation fixed. Thank you!

🇪🇸Spain plopesc Valladolid

Thank you for bringing this up!

🇪🇸Spain plopesc Valladolid

MR created.

🇪🇸Spain plopesc Valladolid

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

🇪🇸Spain plopesc Valladolid

plopesc created an issue.

🇪🇸Spain plopesc Valladolid

Description field added to v3! 🎉

As part of this MR, the schema architecture has been updated.

🇪🇸Spain plopesc Valladolid

Functional MR created

🇪🇸Spain plopesc Valladolid

Nice feature for 3.x branch

🇪🇸Spain plopesc Valladolid

Hello @avpaderno,

I have not heard from the original maintainer along these last 15 days.

I'm still interested in moving forward this module.

Moving the issue to Needs Review to confirm if any other action item is necessary from my side.

Have a great week.

🇪🇸Spain plopesc Valladolid

Postponing on 📌 Store relation with the parent entity in the database instead of the State API Active . Module architecture changes there will affect how tests might need to be addressed.

🇪🇸Spain plopesc Valladolid

Hello @pfrenssen!

I really like the idea brought in this issue. Last week I started to work on a similar upgrade path for Field Inheritance module. You can see how it looks in 🌱 [META] 3.0.0 Plan Active .

Key part of the upgrade would be to integrate the Field Inheritance architecture changes introduced in 📌 Store relation with the parent entity in the database instead of the State API Active , which you actually suggested a while ago.

I'm wondering if we could define a similar approach for Recurring events, where a new 3.x branch is used to define Drupal 10.3+ releases based on Field Inhetance 3.x.

The reason why I preferred to use 10.3 instead of directly a new 11.x branch was to give sites the opportunity to have an stable environment with the new Field Inheritance architecture before moving to 11.x. This approach was taken by Drupal Commerce and the 3.x version. That was very convenient for us in other Commerce based projects, where the migration was taken in 2 steps to reduce friction points.

I'm open to help and collaborate with the migration of this module, given that it's a key requirement in some of our projects. At the same time a review and test of the new FI architecture would be very helpful to ensure that both modules are aligned.

🇪🇸Spain plopesc Valladolid

MR is ready for review. It includes the upgrade path from the old DB structure based on the keyvalue storage to the new entity based approach.

🇪🇸Spain plopesc Valladolid

D7 already reached its EOL. This issue is not relevant anymore.

Thank you for using this module!

🇪🇸Spain plopesc Valladolid

Created an initial MR that more or less work:

Features implemented:

ToDos:

  • Take care of field instantiation at config update event instead of Config Form submit to avoid issues when installing the module or deploying cahnges in the list of entity types
  • Improve upgrade path to only make the widget visible to the enabled bundles
  • Add some kind of confirmation step before submitting the Config form changes indicating that some data might be lost when disabling entity types

Notes:

🇪🇸Spain plopesc Valladolid

I like this approach, and the idea of having a field where the information is stored tied to the entity instead of a separated storage.

I don't see the point of the environment specific storage, as we should consider the relationship between the entities as content, which is not intended to be shared across environments. Or are you talking about scenarios where modules like default_content are in place?

Moving into this would definitely solve 🐛 Field Inheritance data is not removed from the database when the entities are deleted nor the module uninstalled Active and would give other benefits, like using a widget instead of a form alter to handle the inheritance form.

On the other hand, it could be a big BC change for modules and projects depending heavily on this module, like Recurring Events.

I will try to take a look into this for 3.0.0. If I see this is not possible in the timeframe I have to work on this module, I would postpone this one for a future 4.x branch.

Thank you for your proposal!

🇪🇸Spain plopesc Valladolid

I see your point, but not sure how to approach it.

Technically each field_inheritance can depend on a different entity. Event Instance A might use the title of Event Series X and the location of EventSeries Y, hence we need an explicit action for defining them.

That's an edge case, but the approach taken there, even if smart, would not cover all the scenarios.

I was thinking in something more in the line of adding an option in the entity form that says "All the Inheritance from Entity:Bundle X will share the source" and store it as it is in the DB. Then, for entities with that option enabled, those fields will automatically be inherited. Meanwhile, regular fields with explicit assignation would remain as they now, waiting for an explicit assignation.

🇪🇸Spain plopesc Valladolid

Looks good to me.
Marking as RTBC as this will cover a high percentage of the use cases.

Our team will have to use an overridden version of the JS file in any case :)

🇪🇸Spain plopesc Valladolid

Hi!
Hace you tried the v1.2.1 released recently?

It should give support for both Commerce v3 and Drupal 11.

Please let us know if that released does not work for you.

🇪🇸Spain plopesc Valladolid

Thank you very much @qorban!

MR merged

🇪🇸Spain plopesc Valladolid

After asking @owenbush, we agreed that the extra value added by this feature is not worth the effrt to maintain the logic for that form.

This feature will be gone in 3.0.0 for now.

If someone else can provide a working alternative, we will be happy to include it in the module.

Sorry for the inconvenience

🇪🇸Spain plopesc Valladolid

Marking as Fixed since I merged before I realized @lavanyatalwar was reviewing.

Please reopen if you find any issue.

Thank you!

🇪🇸Spain plopesc Valladolid

plopesc created an issue.

🇪🇸Spain plopesc Valladolid

Moving to 3.x branch. Considered as stable blocker.

🇪🇸Spain plopesc Valladolid

MR created

🇪🇸Spain plopesc Valladolid

plopesc created an issue.

🇪🇸Spain plopesc Valladolid

MR merged. Needs to be ported to 3.x.

Thank you

🇪🇸Spain plopesc Valladolid

plopesc created an issue.

🇪🇸Spain plopesc Valladolid

Moving to the 3.x branch

🇪🇸Spain plopesc Valladolid

MR merged. Thank you!

🇪🇸Spain plopesc Valladolid

Thank you for the MR @pfrenssen

Agree with your suggestion and opened a new 3.x branch where the MR points to now.

To ensure that there are no conflicts with this new 3.x branch, bumped the minimum requirement from 10.1 to 10.3.

🇪🇸Spain plopesc Valladolid

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

🇪🇸Spain plopesc Valladolid

Code looks good and MR has been merged.

Thank you!

🇪🇸Spain plopesc Valladolid

Created MR based on a ChainPageContextBuilderInterface that allows for now to define the page context in content entity pages, but opens the door to extend it to other routes.

XB could implement its own PageContextBuilderInterface service and define its own page context SDC component for the top bar.

🇪🇸Spain plopesc Valladolid

After some trial and error with @jcandan, we found that the bug only happens if the phone field is not affected during the edit phase.

If the form element validation is triggered, either modifying the value or just focusing the form element, the country code is moved to the textfield and validation works as expected

🇪🇸Spain plopesc Valladolid

Tested this issue in a 10.4.7 site with both telephone_validation and and telephone_international_widget modules enabled.

Followed the steps to reproduce and I was not able to reproduce the bug under different circumstances.

Attached screen record for reference. https://www.drupal.org/files/issues/2025-05-20/3302638.mp4

I think we can mark this issue as fixed. Marking as RTBC for now :)

🇪🇸Spain plopesc Valladolid

In the same line of the previous comment. We think that would be helpful for individuals and organizations with resources to help to move forward this module to have a list with the issues to be addressed to achieve our common goal.

Thank you!

🇪🇸Spain plopesc Valladolid

We’re using this module in several of our projects, and it would be incredibly helpful to have at least an alpha release to unblock our Drupal 11 migration, even as other issues are being worked on for a stable version.

If you feel the module isn’t ready for an alpha just yet, would you mind creating a “Plan” issue outlining the remaining work needed for a D11-compatible tagged release? That would give us a clearer sense of where to focus contributions.

🇪🇸Spain plopesc Valladolid

Tested the patch and can confirm that it works.

🇪🇸Spain plopesc Valladolid

Moved the previous patch to an MR, so it was possible to install the module's fork via composer to test it.

I can confirm that works as expected. Would be great to have this one merged as a 1st step to have the module ready for D11 as well.

🇪🇸Spain plopesc Valladolid

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

🇪🇸Spain plopesc Valladolid

I would not force to return a price in getBalance()

As demonstrated in 🐛 GiftcardOrderCurrencyValidator assumes that balance is always present Active , it is possible to have giftcards without balance.

Can be reproduced using this code:

$giftcard = $this->entityTypeManager
      ->getStorage('commerce_giftcard')->create([
        'type' => $bundle,
        'code' => $giftcard_code,
        'status' => 1,
        'uid' => $account->id(),
        'stores' => $stores,
      ]);

$giftcard->save();

When creating gift cards programmatically, the validation is not triggered, hence giftcards without balance are technically possible possible.

🇪🇸Spain plopesc Valladolid

You are right... I checked the wrong branch when the issue was created.

My apologies for the noise.

🇪🇸Spain plopesc Valladolid

The issue has been open in the project issue queue for more than 14 days with no response from the main maintainer.

Would you mind to try to contact them and proceed?

Thank you!

🇪🇸Spain plopesc Valladolid

MR created. Take into account this code is not compatible with Commerce v2.

🇪🇸Spain plopesc Valladolid

Updated MR to support Commerce v3

Production build 0.71.5 2024