Also, support requests are generally not answered from this queue:
With the abundance of support available in Drupal Answers, the Drupal focused Stack Exchange site, we are no longer answering support requests in the issue queue.
Support requests opened here will be closed (won't fix), so please search for existing answers or post new questions at:
https://drupal.stackexchange.com (using the drupal-commerce tag)
The text is still in the template? The "Thank you for your order!" text. Did you rebuild your caches after making changes to the template?
What's preventing you from adding VBO to your cart view? Commerce core doesn't depend on VBO to limit the dependencies to other contrib modules, but adding Operations to your view is like a 5 minutes task :).
Same issue here. Any chance for a new tagged release with this fix @jsacksick?
We'll tag the release whenever we are ready, we aren't quite ready to tag it yet.
It is a warning that isn't really consequential IMO... If the warning is really bothering you, you can apply the patch in the meantime.
Can you be more specific on:
Steps to reproduce
Have a route that matches the pattern of state_transition_form but does not pass a state field in field_name.
Why would you define a state-transition-form link template that does not respect the pattern? I don't think it would work? What is the use case we're addressing here? I believe you're trying to do something that is not supported, so I don't believe this fix make sense?
I was thinking of updating the following method:
/**
* Refreshes the order and applied promotions.
*
* @param \Drupal\commerce_order\Entity\OrderInterface $order
* The order entity.
*/
private function reapplyPromotions(OrderInterface $order) {
to accept a second parameter $promotion_id so applying/removing a coupon for example only remove adjustments previously added by the promotion ID referenced by the coupon.
This avoid messing with promotion adjustments previously added by other promotions (in case promotions are stacked). This is particularly useful for placed orders.
But then I realized there are other issues to take into account. The tax might need to be reapplied / recalculated as well... So perhaps as a v1 it's safer to only support draft orders as we could end up recalculating promotions for placed orders and end up with wrong tax adjustments... The tax is always calculated post promotion.
The other solution would be to reapply taxes as well...
Will keep digging tomorrow.
This needs additional work as it doesn't work properly for placed orders.
So I think we need to make a decision, either we try supporting placed orders properly, and for that I think we need to explicitly save order items... Or we don't make the action available for non draft orders...
If we do limit this to draft orders only, then the code can be simplified to simply save the order, as draft orders are automatically refreshed on order save.
I actually do prefer the approach of manually invoking the promotion order processor, as this is basically doing a "scoped" order refresh, but the current logic doesn't work for placed orders.
Also, we need additional test coverage to test various use cases.
This needs a reroll, but also not fully sure I understand what we're fixing here.
This looks mostly good... The only thing I'm worried about is the fact that we remove all promotion adjustments. This is ok I'd say in most cases, but in the scenario where a placed order is updated, this might cause a promotion previously applied (without coupons) to be removed and not reapplied.
Perhaps an edge case we can live with as we can assume that if a merchant adjusts the coupons, this should take predecence over previously applied promotions.
Also, we're still injecting the order refresh service and not using it.
@tbkot: I'm in favor of making the service final, remove the interface and mark the service as @internal. I'd prefer to be able to break that serivce if needed by adding or removing parameters etc etc... I don't think we should have to worry about it, as this was created to workaround a bug, this isn't supposed to be a public API.
Let's rename the action to "Manage coupons" and also remove the "Add order item" local action.
@tbkot: Also do we have tests actually reproducing the bug that we're fixing? Ensuring the profile is correctly owned by the user?
@tbkot: Can we move the procedural functions introduced to a service?
For example:
namespace Drupal\profile;
use Drupal\profile\Entity\ProfileInterface;
use Drupal\user\UserInterface;
class ProfileUserSyncer {
/**
* Profiles queued for users.
*
* @var array
*/
protected array $preparedProfiles = [];
/**
* Add a profile that should be saved once the user exists.
*/
public function addPreparedProfile(UserInterface $user, ProfileInterface $profile): void {
$uuid = $user->uuid();
$this->preparedProfiles[$uuid][] = $profile;
}
/**
* Get and reset prepared profiles for a user.
*
* @return \Drupal\profile\Entity\ProfileInterface[]
*/
public function flushPreparedProfiles(UserInterface $user): array {
$uuid = $user->uuid();
$profiles = $this->preparedProfiles[$uuid] ?? [];
unset($this->preparedProfiles[$uuid]);
return $profiles;
}
/**
* Save a profile immediately or queue if user is new.
*/
public function saveProfile(UserInterface $user, ProfileInterface $profile): void {
if ($user->isNew()) {
$this->addPreparedProfile($user, $profile);
}
else {
$profile->setOwnerId($user->id());
$profile->setPublished();
$profile->save();
}
}
}
OOP Preprocess hooks are only supported from D11.2. See https://www.drupal.org/node/3496491 → .
Also even if that was working, I noticed a typo (preprocess_HOOK) instead of the actual hook.
As a side note, interesting that we didn't use an event subscriber for reacting to the order deletion.
Closing this as ✨ OrderAssignment::assign() prevents custom order email addresses Fixed was implemented.
@damienmckenna: Thank you for this. Could you open an MR against 3.x, 3.0.x isn't the right branch.
Merged, thanks, there is a bug with the Contribution record page which doesn't let me properly assign the contribution...
Actually, I'm having second thoughts about this and now considering removing the old update hooks removal. The reason being that this essentially force people to upgrade to Commerce 3 from Commerce 2.40 (or the last version that contained the highest update hook).
This can be problematic and cause a broken schema so I'm now thinking that we should revert the change.
We can now remove the applied patch from the composer.json as it got committed.
This patch is 95% ready, good job. Feedbacks were provided internally.
Retitling, for clarity as this task isn't actually about implementing the modal itself since all the logic lives in Commerce Shipping.
There is still a lot to do to be compatible with Commerce 3.
variationType that became variationTypes etc... Still plenty of errors... Was expecting this to be more straightforward.
hm... Why do we need to set the changedTime()? This should be overridden on presave() no? What happened with the merge commit? The diff seems messed up.
Ok thanks for reporting back, makes sense, I mixed up the ChangedItem logic which sets the changed time on presave(). CreatedItem doesn't have such logic and I can understand why.
Could you open a merge request please?
Also, we can simply do the following instead:
if (!$cart->getStore()?->isPublished()). There is no need for a separate condition.
Ok the only reason why this route would be needed I think is the access callback which depends on commerce_checkout.
If that is the only reason, perhaps we should fix the access callback and maybe make it require order update access instead?
To be discussed.
Adding the needs tests tag to ensure we have tests coverage for showing the payment information.
I think the gaps should be covered by a contrib commerce_marketplace module or so and eventually contributed back later.
There are several issues opened around permissions etc... But since the marketplace use case isn't what most people need/want, we didn't make the marketplace use case a priority.
This isn't possible. In 95% of the sites I'm working on products are created/administer by users that do not own the store (merchandisers etc...).
@tbkot: I've pushed several changes to the MR but there is still room for improvement:
- Can we leverage the Commerce component introduced in this commit? https://git.drupalcode.org/project/commerce/-/commit/b26d4e59323809efb6df3f65b383ae1af21067e0
- There is no need to display the panel (and numbers at the bottom if there is a single shipment).
- Can we rename the button to "Add another shipment" in case there is already one shipment?
- We don't have handling to display "locked" in case the order is completed and disallow editing shipments.
- Reported in the Commerce queue, but we should find a way for Commerce to still support Commerce shipping 2.x and support both "shipping_information" and "shipments".
@tbkot: I think we should find a way for Commerce to support both.
If the order.shipments field is present, we use that, otherwise we check if order.shipping_information is present.
This way we shouldn't need to force a minimum requirement on Commerce shipping 3.0. Plus it is kind of annoying as we have a circular dependency. (We need the latest Commerce, and Commerce conflicts with prior versions of Shipping).
This isn't ideal.
Marking this as needs work as there are conflicts with the MR.
I see multiple problems with the default widget provided.
The main problem being that the widget is provided by Commerce core but we're accessing the payment gateway storage (which might not exist if the commerce_payment module is not installed).
Also the provider value is usually ("<$payment_gateway_id>|(e.g: "test"|"live")). The code doesn't really account for that.
Furthermore, not sure what we should do in terms of access, but we definitely shouldn't allow a random user to mess with the remote IDS that are stored.
There are also a few coding standard issues, and there is no need to set the gateway storage when we can just inject the entity type manager and instantatiate the gateway storage when we needed (rather than straight from the constructor). Also, we tend to avoid overridding the parent constructor to avoid breaking in case the parent definition changes).
Setting this as "needs work" even though I'm not sure what the proper solution is... The formatter is provided by Commerce core, so it doesn't feel right for the widget to be defined by commerce_payment.
Also, the remote ID field was probably meant at first to be used for storing remote IDS of any kind (I presume), but in practice we ended up storing only payment gateway remote IDS.
Ok I'm now exposing the "Add payment" button for all payment options (not just onsite gateways).
A "payment_received" checkbox was added (label "Mark as received") for manual gateways.
Finally, the payment state isn't set to early as most of the createPayment() implementation out there expect a payment with a "new" state passed.
I believe we need to update the comments. Right now it says:
// Do not invalidate 4xx-response cache tag, because profiles in most cases
// are not affected by possibly stale 404 pages as checkout is not cached for
// anonymous users.
Why does this mention checkout?
hm... Shouldn't we nullify the created time instead on createDuplicate()? So it's set on save properly automatically? If the form is submitted much later, then the created time wouldn't be accurate?
I'm guessing we need to do the same for products as well?
Now that ✨ Implement billing information edit modal Active is in, this needs a re-roll and adjustments.
Note that no actions buttons are shown when the payment gateway isn't onsite, so no way to submit a payment for manual gateways etc...
How is that a bug report? We won't be renaming the PaymentMethodAddForm class as this would be a breaking change for implementations extending it or using it.
This is more like a base class / example class used in test payment gateway plugins and used by the commerce_payment_example module.
Payment gateways typically define their own Payment method forms.