Skip to content
This repository was archived by the owner on Feb 19, 2025. It is now read-only.

Fix specs #104

Merged
merged 8 commits into from
Apr 16, 2024
Merged

Fix specs #104

merged 8 commits into from
Apr 16, 2024

Conversation

mamhoff
Copy link
Contributor

@mamhoff mamhoff commented Apr 9, 2024

The removal of the legacy promotion system from the main Solidus repo exposed some hidden dependencies. This PR removes those hidden dependencies.

mamhoff added 4 commits April 9, 2024 11:06
We were using Spree helpers before; those don't exist anymore.

Also we missed importing the PromotionsHelper, and implicitly relied on
the one provided by Solidus.
In some spots we were still relying on Solidus partials, even though we
have them in the app.
Previously, the activation UI for promotions was based on complicated
Javascript. This change allows us all the same options, and the user
will be warned by validation errors if they try to create a promotion
with both a code and the apply automatically option set. For existing
promotions, `apply_automatically` will be disabled if either a path is
set or a promo code is set.
@@ -1,5 +1,5 @@
<% admin_breadcrumb(link_to plural_resource_name(Spree::Promotion), spree.admin_promotions_path) %>
<% admin_breadcrumb(link_to @promotion.name, spree.admin_promotion_path(@promotion.id)) %>
<% admin_breadcrumb(link_to plural_resource_name(Spree::Promotion), solidus_friendly_promotions.admin_promotions_path) %>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this change too?

<% admin_breadcrumb(link_to plural_resource_name(Spree::Promotion), spree.admin_promotions_path) %>
<% admin_breadcrumb(link_to @promotion.name, spree.admin_promotion_path(@promotion.id)) %>
<% admin_breadcrumb(link_to plural_resource_name(Spree::Promotion), solidus_friendly_promotions.admin_promotions_path) %>
<% admin_breadcrumb(link_to @promotion.name, solidus_friendly_promotions.admin_promotion_path(@promotion.id)) %>
<% admin_breadcrumb(plural_resource_name(Spree::PromotionCodeBatch)) %>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same

<% end %>
</div>

<% if f.object.new_record? %>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add || present? here?

mamhoff added 4 commits April 9, 2024 15:35
This got by undetected because the spec was using the wrong model name.
If this promotion is activated by a path, it makes sense to be able to
change it.
Copy link
Collaborator

@davecandlescience davecandlescience left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work

@mamhoff mamhoff merged commit 10b7697 into main Apr 16, 2024
4 checks passed
@mamhoff mamhoff deleted the fix-specs branch April 16, 2024 14:35
mamhoff added a commit that referenced this pull request Jul 1, 2024
mamhoff added a commit that referenced this pull request Oct 25, 2024
mamhoff added a commit that referenced this pull request Oct 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants