🇩🇪Germany @ktpm

g4mbini → credited ktpm → .

ktpm → created an issue.

Not sure if I should post this here or in #3188336: Updates to string causes over-write of fallback language → as both are related, but since this issue is still open, let's go with this.

Currently, if you try to import translations to a language, which has parent languages, the translations of the parent language may be overridden, when using drush locale:import with any --override option. This happens, if there is no string item for the target language yet. This probably also occurs when using the import form in the backend, but I did not explicitly test it that way.

Example:
EN as parent language
Set any language as child language, like DE.

How it happens:
- The StringDatabaseStorageDecorator::dbStringSelect() wants to retrieve fallback candidates for the db select.
- The candidate selection is done in the language_fallback_candidates_alter hook. EN is returned as candidate.
- DE & EN are passed to the db select. String item for DE does not exist, hence the query returns the EN item.
- PoDatabaseWriter::importString() SHOULD go to the else part ("No such source string in the database yet.") but gets EN string.
- Because override option is set, string item gets overwritten/updated.

As the candidate selection part is done in the alter hook, I believe the check should be done there, as I did in the attached patch file. Not too sure about it though. So I added some checks there if the current process is being run via cli (drush), if there is an actual batch being processed, and if the batch is a locale translate import, to be as specific as possible about this. There may be better ways to do this, but it helped me to avoid this issue.

ktpm → created an issue.

#10 also fixed it for me, with the same code as #33. I seem to have missed that before!

For anyone stumbling on this:

Until this is fixed someday, you can override the file path(s) of the library in your theme's info.yml:

libraries-override:
    fiu/magnific:
    js:
      /libraries/Magnific-Popup/dist/jquery.magnific-popup.min.js: /libraries/magnific-popup/dist/jquery.magnific-popup.min.js
    css:
      theme:
        /libraries/Magnific-Popup/dist/magnific-popup.css: /libraries/magnific-popup/dist/magnific-popup.css

Reference: https://www.drupal.org/node/2497313 →

Change merged in current dev. Thanks!

I'm still getting

commerce_order_report entity type :
The Address field needs to be updated.

with patch #30.

ktpm → created an issue.

This is precisely what we're doing?

Yes, but only in update hook, where it's shown once. I thought about a message repeatedly shown in the backend, e.g. when editing a pricelist.

Do you need a schema with this constraint at all?

If yes: Since you cannot decide which of the duplicates should be removed, maybe you could provide a form, that can be submitted once, if the conditions for setting the schema are met. But I don't know if there are best practices for adding schemas afterward, like in this case.

But maybe it's sufficient to display a message like "We have duplicates detected, please check" and let the user deal with it? Just a thought.

Using group by in MySQL with only_full_group_by requires all columns to be part of the group or being aggregated in any way, like count(). The column ID was not aggregated like that, thus the resulting error.

The attached patch should do the trick.

I don't understand the purpose of the update hooks, though.

commerce_pricelist_update_8206(): If there are any results, the storage schema will not be set and the message "Update to the commerce_pricelist_item schema was skipped due to duplicate prices." will be returned instead.

commerce_pricelist_update_8207(): Will try to set the storage schema regardless of what commerce_pricelist_update_8206() did. As I see it, assumed the query in update 8206 returned a result before, update 8207 WILL catch an exception "Integrity constraint violation: Duplicate entry" which is returned as a message.

In this case the update does essentially nothing and returns error messages, which gives the impression that something is missing or didn't work out, but the update cannot be repeated without further ado.

I think providing an informal message or view, in case of duplicate products with same quantity and currency, would be a better approach and gives the user the opportunity to act on it.

Hi guy_schneerson,

I think commerce_stock_enforcement_get_context() should be moved to the context creator trait as it (presumably) always returns a store where the current implementation does not.

This can result in an error page if we assume to have stores A, B and C, where A is the default store, a product has selected B and C, and then you try to edit a product variation.

Using the SelectStoreTrait saves a few lines of code but requires a try/catch. Please check my approach in the MR.

ktpm → made their first commit to this issue’s fork.

ktpm → created an issue.

Hi,

thank you for your suggestion. I understand where this comes from.
The name recruiting comes from the core entity, the recruitment, which everything in the module revolves around. We wanted to apply the same pattern as e.g. commerce_shipping, where the core entity is called "Shipment", but the module is called "shipping". We also thought of the module in terms of "getting a new customer on board of your shop", so "recruitment" seemed pretty accurate to us.
Of course referral could have been fitting as well, but since this module is already up for over 3 years, we will want to leave the name as it is. Thank you for your understanding.

I have merged and released the changes now with v8.2.0.

Updated documentation in readme, released with v8.1.4 & v8.2.0

I have checked the module against Drupal 10.1, and had to make a few adjustments:

- the event dispatcher has switched their argument positions: event name <---> event
- entity queries are now required to set access check
- Symfony\Component\EventDispatcher\Event does not exist anymore, replaced it with Drupal\Component\EventDispatcher\Event

Plus, because of the drupal/dynamic_entity_reference^3 dependency, this version will also require Drupal 10 and will be released as 8.2.0.

Before I merge MR!1 I appreciate it if you can check once more and let me know if these changes work for you.

Hi, we are using simple cron with the watchdog mailer module because we want to be notified if a cron fails for any reason.
The mailer module sends logs from warning level and anything more severe though.
By removing the warning log this change does work for us. Thanks.

Hi,
I added an MR for the 1) easy way.

ktpm → made their first commit to this issue’s fork.

Hi all,

thank you addressing this topic! I will check the MR and make a release soon.

Glad I could help!

Hi,

the code is built as "[campaign-option-code]--[user]", where user is the UID by default. It is NOT the username. But the user account does have its own code field if you want them to have something more fancy than "xyz--2".

Also, you can add the "Recruiter campaigns sharing link block" somewhere in the page, which will list all campaigns that the user is part of with a "Copy Link" button for each product.

Hi,

thanks for reaching out!

Multi value on the product field is currently not supported as I need to match exactly one code with one specific product to determine where to redirect. But you can allow the recruiter to receive the bonus from multiple items by adding all necessary products as seperate campaign-options to the same campaign and enabling the option "Apply bonus from any matching option". You'll find this option when editing the campaign.

Does this help you?

Hi,

And I am stumped on how to actually enable the module

I noticed a missing dependency to commerce_cart that was added in the last release. Please try the updated version.

drush cr
Drupal\Component\Plugin\Exception\PluginNotFoundException: The
"commerce_plugin_item:commerce_recruiting_bonus_resolver" plugin does not
exist.

The module is not installed yet.

I cannot reproduce this issue as plugins of disabled modules are ignored and should not produce this exception at all, so this issue may be out of scope for this module.

If the issue persists with the updated version, please provide more information about the steps for me to reproduce.

Patch was removed from composer.json

Hi,

thank you for pointing that out! I will remove the patch sometime later when releasing a new version.