Merge solidus_starter_frontend in as "storefront"#6468
Conversation
In order to use the main one in our organization's .github repository.
…template Remove custom PR template
…ch-specs Polish search autocomplete specs
In the order details page shown for the confirmation step and the order complete page the billing address should be more visually related to the payment.
Also show the payment state, near the payment.
Code readability and (basic) UX improvements
Probably a legacy of an older layout.
For parity with the new card / payment method partials in which the radio buttons are external to the partials provided by the payment methods.
In a real world scenario this would probably hold instructions on where to send the check. This work both as a placeholder and to visually balance the check with other payment methods.
Only useful to remember how the numbers are distributed while testing.
The triple level of radio buttons that was previously used was very confusing and broken. Instead of trying to fix it seems better to present the customer a single list of options for making the payment. The list gives precedence to wallet payments as they require less clicks. If no wallet payments are available the first payment method is selected. Whenever a radio is selected elements from the other list are deselected, giving the illusion of having a single list. Each existing and new payment method have clear and distinct areas for the input label and for the details. This gives more symmetry between existing and new payments partial implementations and a cleaner visual layout.
…anup Payment step cleanup
enable localizing shopping cart review page
feat: localizing cart review page
Otherwise, payment iframes will still be clickable Co-authored-by: Elia Schito <elia@schito.me> Co-authored-by: Francesco Aiello <francesco.aiello@hey.com>
…ointer-events-none-to-disabled-fields Block clicks on disabled form elements
- Only render the currency microdata when the price is present - Keep the cache block in the products grid - Add specs for rendering with/without a taxon Co-Authored-By: Safa <aballaghsafa@gmail.com>
Useful for newly created stores and for testing extensions. Only track coverage on Ruby 3.2 which supports ERB files.
Track spec coverage with codecov
They were commented out for quickly trying out an alternative configuration but never removed before the merge of solidusio#329.
Restore commented out CI jobs
This makes it more consistent and recognizable.
| gem "rails-controller-testing", "~> 1.0.5" | ||
| gem "rspec-activemodel-mocks", "~> 1.1.0" | ||
|
|
||
| gem "factory_bot", ">= 4.8" |
There was a problem hiding this comment.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
| gem_group :development, :test do | ||
| gem "rspec-rails" | ||
| gem "rails-controller-testing", "~> 1.0.5" | ||
| gem "rspec-activemodel-mocks", "~> 1.1.0" |
There was a problem hiding this comment.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
|
|
||
| gem_group :development, :test do | ||
| gem "rspec-rails" | ||
| gem "rails-controller-testing", "~> 1.0.5" |
There was a problem hiding this comment.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
| end | ||
|
|
||
| gem_group :development, :test do | ||
| gem "rspec-rails" |
There was a problem hiding this comment.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
| gem "capybara-screenshot", "~> 1.0" | ||
|
|
||
| gem "simplecov", require: false | ||
| gem "simplecov-cobertura", require: false |
There was a problem hiding this comment.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
| gem "tailwindcss-rails", "~> 3.0" | ||
|
|
||
| gem_group :test do | ||
| # We need to add capybara along with a javascript driver to support the provided system specs. |
There was a problem hiding this comment.
Metrics/LineLength: Line is too long. [98/80]
| gem "responders" | ||
| gem "solidus_support", ">= 0.12.0" | ||
| gem "view_component", "~> 3.0" | ||
| gem "tailwindcss-rails", "~> 3.0" |
There was a problem hiding this comment.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
|
|
||
| gem "responders" | ||
| gem "solidus_support", ">= 0.12.0" | ||
| gem "view_component", "~> 3.0" |
There was a problem hiding this comment.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
| end | ||
|
|
||
| gem "responders" | ||
| gem "solidus_support", ">= 0.12.0" |
There was a problem hiding this comment.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
| generate "solidus:auth:install" | ||
| end | ||
|
|
||
| gem "responders" |
There was a problem hiding this comment.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
| with_log["installing gems"] do | ||
| unless Bundler.locked_gems.dependencies["solidus_auth_devise"] | ||
| bundle_command "add solidus_auth_devise" | ||
| generate "solidus:auth:install" |
There was a problem hiding this comment.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
|
|
||
| with_log["installing gems"] do | ||
| unless Bundler.locked_gems.dependencies["solidus_auth_devise"] | ||
| bundle_command "add solidus_auth_devise" |
There was a problem hiding this comment.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
| end | ||
|
|
||
| with_log["installing gems"] do | ||
| unless Bundler.locked_gems.dependencies["solidus_auth_devise"] |
There was a problem hiding this comment.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
| source_paths.unshift(templates_dir) | ||
| end | ||
|
|
||
| with_log["installing gems"] do |
There was a problem hiding this comment.
Metrics/BlockLength: Block has too many lines. [29/25]
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
|
|
||
| templates_dir = Pathname.new(repo_dir).join("storefront", "templates") | ||
| else | ||
| templates_dir = Pathname.new(File.dirname(__FILE__)).join("templates") |
There was a problem hiding this comment.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
| unless match | ||
| say_status :error, shell.set_color( | ||
| "Could not parse storefront template URL: #{__FILE__}\n" \ | ||
| "Expected shape: https://github.com/<owner>/<repo>/raw/<ref>/storefront/template.rb", |
There was a problem hiding this comment.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
Metrics/LineLength: Line is too long. [93/80]
| if __FILE__.match?(%r{\Ahttps?://}) | ||
| require "uri" | ||
| url_path = URI.parse(__FILE__).path | ||
| match = url_path.match(%r{\A/([^/]+)/([^/]+)/raw/(.+?)/starter_frontend/template\.rb\z}) |
There was a problem hiding this comment.
Metrics/LineLength: Line is too long. [92/80]
| require "securerandom" | ||
|
|
||
| if __FILE__.match?(%r{\Ahttps?://}) | ||
| require "uri" |
There was a problem hiding this comment.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
| # In that case, use `git clone` to download them to a local temporary dir. | ||
| with_log["fetching remote templates"] do | ||
| require "shellwords" | ||
| require "securerandom" |
There was a problem hiding this comment.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
| # invoked remotely via HTTP, that means the files are not present locally. | ||
| # In that case, use `git clone` to download them to a local temporary dir. | ||
| with_log["fetching remote templates"] do | ||
| require "shellwords" |
There was a problem hiding this comment.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
b7f4ad6 to
50d6111
Compare
| # copy_file and template resolve against our source files. If this file was | ||
| # invoked remotely via HTTP, that means the files are not present locally. | ||
| # In that case, use `git clone` to download them to a local temporary dir. | ||
| with_log["fetching remote templates"] do |
There was a problem hiding this comment.
Metrics/BlockLength: Block has too many lines. [29/25]
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
| "You are installing solidus_storefront on an outdated Ruby version.\n" \ | ||
| "Please keep in mind that some features might not work with it.", :bold | ||
| ), :red | ||
| exit 1 if auto_accept || no?("Do you wish to proceed?") |
There was a problem hiding this comment.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
| if Gem::Version.new(RUBY_VERSION) < Gem::Version.new("2.7") | ||
| say_status :unsupported, shell.set_color( | ||
| "You are installing solidus_storefront on an outdated Ruby version.\n" \ | ||
| "Please keep in mind that some features might not work with it.", :bold |
There was a problem hiding this comment.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
| exit 1 | ||
| end | ||
|
|
||
| if Gem::Version.new(RUBY_VERSION) < Gem::Version.new("2.7") |
There was a problem hiding this comment.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
| if Rails.gem_version < Gem::Version.new("7.0") | ||
| say_status :error, shell.set_color( | ||
| "You are trying to install solidus_storefront on an outdated Rails version.\n" \ | ||
| "This installation attempt has been aborted, please retry using at least Rails 7.", :bold |
There was a problem hiding this comment.
Style/StringLiterals: Prefer single-quoted strings when you don't need string interpolation or special symbols.
Metrics/LineLength: Line is too long. [95/80]
| selects.forEach(sel => sel.setAttribute('disabled', true)); | ||
| } else { | ||
| addressInputs.forEach(addressInput => addressInput.style.display = 'block'); | ||
| inputs.forEach(input => input.removeAttribute('disabled')); |
There was a problem hiding this comment.
'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').
| inputs.forEach(input => input.setAttribute('disabled', true)); | ||
| selects.forEach(sel => sel.setAttribute('disabled', true)); | ||
| } else { | ||
| addressInputs.forEach(addressInput => addressInput.style.display = 'block'); |
There was a problem hiding this comment.
'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').
| if (order_use_billing.checked) { | ||
| addressInputs.forEach(addressInput => addressInput.style.display = 'none'); | ||
| inputs.forEach(input => input.setAttribute('disabled', true)); | ||
| selects.forEach(sel => sel.setAttribute('disabled', true)); |
There was a problem hiding this comment.
'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').
| const selects = document.querySelectorAll('#shipping .address-inputs select'); | ||
| if (order_use_billing.checked) { | ||
| addressInputs.forEach(addressInput => addressInput.style.display = 'none'); | ||
| inputs.forEach(input => input.setAttribute('disabled', true)); |
There was a problem hiding this comment.
'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').
| const inputs = document.querySelectorAll('#shipping .address-inputs input'); | ||
| const selects = document.querySelectorAll('#shipping .address-inputs select'); | ||
| if (order_use_billing.checked) { | ||
| addressInputs.forEach(addressInput => addressInput.style.display = 'none'); |
There was a problem hiding this comment.
'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').
| function update_shipping_form_state(order_use_billing) { | ||
| const addressInputs = document.querySelectorAll('#shipping .address-inputs'); | ||
| const inputs = document.querySelectorAll('#shipping .address-inputs input'); | ||
| const selects = document.querySelectorAll('#shipping .address-inputs select'); |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
|
|
||
| function update_shipping_form_state(order_use_billing) { | ||
| const addressInputs = document.querySelectorAll('#shipping .address-inputs'); | ||
| const inputs = document.querySelectorAll('#shipping .address-inputs input'); |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
| }); | ||
|
|
||
| function update_shipping_form_state(order_use_billing) { | ||
| const addressInputs = document.querySelectorAll('#shipping .address-inputs'); |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
| update_shipping_form_state(orderUseBilling); | ||
| }); | ||
|
|
||
| function update_shipping_form_state(order_use_billing) { |
There was a problem hiding this comment.
Function declarations should not be placed in blocks. Use a function expression or move the statement to the top of the outer function.
| element.dispatchEvent(new Event('change')); | ||
| }); | ||
|
|
||
| const orderUseBilling = document.getElementById('order_use_billing'); |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
| stateSelect.innerHTML = ''; | ||
| const statesWithBlank = [{ name: '', id: ''}].concat(states); | ||
| statesWithBlank.forEach(state => { | ||
| const selectOption = document.createElement('option'); |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
| const selected = parseInt(stateSelect.value); | ||
| stateSelect.innerHTML = ''; | ||
| const statesWithBlank = [{ name: '', id: ''}].concat(states); | ||
| statesWithBlank.forEach(state => { |
There was a problem hiding this comment.
'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').
| if (states.length > 0) { | ||
| const selected = parseInt(stateSelect.value); | ||
| stateSelect.innerHTML = ''; | ||
| const statesWithBlank = [{ name: '', id: ''}].concat(states); |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
| const stateInput = stateContainer.querySelector('input'); | ||
|
|
||
| if (states.length > 0) { | ||
| const selected = parseInt(stateSelect.value); |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
| const states = stateData.states; | ||
|
|
||
| const stateSelect = stateContainer.querySelector('select'); | ||
| const stateInput = stateContainer.querySelector('input'); |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
| function update_shipping_form_state(order_use_billing) { | ||
| const addressInputs = document.querySelectorAll('#shipping .address-inputs'); | ||
| const inputs = document.querySelectorAll('#shipping .address-inputs input'); | ||
| const selects = document.querySelectorAll('#shipping .address-inputs select'); |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
|
|
||
| function update_shipping_form_state(order_use_billing) { | ||
| const addressInputs = document.querySelectorAll('#shipping .address-inputs'); | ||
| const inputs = document.querySelectorAll('#shipping .address-inputs input'); |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
| }); | ||
|
|
||
| function update_shipping_form_state(order_use_billing) { | ||
| const addressInputs = document.querySelectorAll('#shipping .address-inputs'); |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
| update_shipping_form_state(orderUseBilling); | ||
| }); | ||
|
|
||
| function update_shipping_form_state(order_use_billing) { |
There was a problem hiding this comment.
Function declarations should not be placed in blocks. Use a function expression or move the statement to the top of the outer function.
| element.dispatchEvent(new Event('change')); | ||
| }); | ||
|
|
||
| const orderUseBilling = document.getElementById('order_use_billing'); |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
| stateSelect.innerHTML = ''; | ||
| const statesWithBlank = [{ name: '', id: ''}].concat(states); | ||
| statesWithBlank.forEach(state => { | ||
| const selectOption = document.createElement('option'); |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
| const selected = parseInt(stateSelect.value); | ||
| stateSelect.innerHTML = ''; | ||
| const statesWithBlank = [{ name: '', id: ''}].concat(states); | ||
| statesWithBlank.forEach(state => { |
There was a problem hiding this comment.
'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').
| if (states.length > 0) { | ||
| const selected = parseInt(stateSelect.value); | ||
| stateSelect.innerHTML = ''; | ||
| const statesWithBlank = [{ name: '', id: ''}].concat(states); |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
| const stateInput = stateContainer.querySelector('input'); | ||
|
|
||
| if (states.length > 0) { | ||
| const selected = parseInt(stateSelect.value); |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
50d6111 to
afe8e98
Compare
jarednorman
left a comment
There was a problem hiding this comment.
I can't approve this because I own the PR, even though @AlistairNorman and @forkata did most of the work, but I approve of it.
@adammathys can you take a look at this?
|
I'm just checking a couple things, so we should hold off on merging until I've confirmed. Maybe found a bug. |
|
I have, in fact, found a bug. |
SimpleCov must start before the app is loaded (Ruby's Coverage only tracks files required after Coverage.start), so we load it from the top of the generated spec_helper.rb rather than from spec/support, which is required after rails_helper has already (eager-)loaded the app.
None of the paypal controllers in this extension had tests, which is causing the code coverage to be under the solidus required limit. This should get us to the minimum 89% code coverage to get the build green. Co-authored-by: Adam Mueller <adam@super.gd> Co-authored-by: Alistair Norman <alistair@super.gd> Co-authored-by: Noah Silvera <noah@super.gd>
5dc4551 to
c4a869b
Compare
System specs default to headless Chrome. Setting CAPYBARA_DRIVER=selenium_chrome switches the Selenium driver to headed Chrome so you can watch specs run in a real browser while debugging. Co-authored-by: Adam Mueller <adam@super.gd> Co-authored-by: Alistair Norman <alistair@super.gd> Co-authored-by: Noah Silvera <noah@super.gd> Co-authored-by: Jared Norman <jared@super.gd>
The rename commit updated the error message and clone paths in storefront/template.rb but missed the URL-parsing regex, which still required the path to end in starter_frontend/template.rb. The installer now generates URLs ending in storefront/template.rb, so every remote install would abort with the 'Could not parse' error. CI never caught this because it applies the template from a local path via SOLIDUS_STOREFRONT_TEMPLATE, which skips the URL parser entirely.
In CI the generated app lives under $RUNNER_TEMP while the repo checkout lives under $GITHUB_WORKSPACE, so the ResultAdapter remap target (SimpleCov.root/../storefront/templates) never exists and the remap never fired. Coverage is still attributed to storefront/templates/** on Codecov because the cobertura report contains app-relative paths (app/controllers/...) that Codecov's path fixing suffix-matches against the repo tree. Verified against the report Codecov ingested for 1f98667. The ERB trailing-line trimming was equally dead: this port of the coverage setup dropped enable_coverage_for_eval, so no .erb files appear in the results at all.
Everything else in .dockerdev derives paths from $APP_USER, so this was the only leftover.
c4a869b to
a355f7f
Compare

This PR merges solidus_starter_frontend into the monorepo as solidus_storefront, preserving its full commit history via a git subtree merge.
What's in the box
Spree.solidus_version. Released versions get theirvX.Ypatch branch, prereleases getmain. No more manual bumping a ref. 🙂git clone. Remote install work with the monorepo URLs. Existing sites point at the old repo.solidus_installerportion of the CI now runs the storefront's full test suite against a fresh app using the in-tree version of Solidus. (Previously, we tested against released gems.)StoreController#cart_linkNote: Under no circumstances should this PR be rebased or squashed. The subtree merge means it needs to merged in with a merge commit.