Skip to content
This repository has been archived by the owner on Sep 10, 2019. It is now read-only.

stock_location_decorator breaks base seeding scripts #59

Open
tblanchard opened this issue Apr 17, 2018 · 3 comments
Open

stock_location_decorator breaks base seeding scripts #59

tblanchard opened this issue Apr 17, 2018 · 3 comments

Comments

@tblanchard
Copy link

Solidus seed scripts create a default stock_location with just a name 'default'. It is now no longer possible to initialize a new solidus instance with this gem bundled because of these validation rules.

This has caused me way more grief than having a stock location without address info would.

@forkata
Copy link
Contributor

forkata commented Apr 18, 2018

Hi @tblanchard, thanks for reporting this issue. Indeed the base seed data does not contain address information and I can see how the validations added by this extension would cause errors. Unfortunately that information is required for building the origin information for shipments.

There are two options I see here, one would be to remove the validations and raise an exception during runtime if a shipment originates from a stock location without address information. This is not ideal because an admin can edit a stock location and remove the necessary address information which would cause getting rates for shipments to start failing for users. The second option would be to make a change to the seed data to add a dummy address, however that doesn't seem correct either.

I will investigate if there is a way for an extension to override the default seeds in order to avoid issues like this.

@tblanchard
Copy link
Author

I worked around it with a decorator containing the following:

  # remove validation rules introduced by active_shipping

  %i[address1 city zipcode country_id].each do | ea |
    _validators[ea].find do |v|
      v.is_a? ActiveRecord::Validations::PresenceValidator
    end.attributes.delete(ea)
  end

  def state_id_or_state_name_is_present
    true
  end

but it is rather inconvenient.

Another worthwhile question is to ask why seeding even bothers to create a default stock_location. It is useless and I end up deleting it and making a new one anyhow, but this is perhaps an issue for the core developers.

@jchapin
Copy link

jchapin commented Sep 5, 2018

I ran into this issue today when my pre-push hook alerted me to a test failure with:

ActiveRecord::RecordInvalid: Validation failed: Street Address can't be blank, City can't be blank, Zip can't be blank, Country can't be blank, State name can't be blank

Another possible solution:

Since the core seed task was part of our current CI / automated deployment I opted to pull the content following out of db/seeds.rb

Spree::Core::Engine.load_seed if defined?(Spree::Core)

and replace it with the content of Solidus' core db seeds file directly.

https://github.com/solidusio/solidus/blob/master/core/db/seeds.rb

I then created a default/spree folder in the project's db directory and filled it with the default seed files from the gem, but I replaced the content in db/default/spree/stock_locations.rb with the information from my client's warehouses. That allowed me to keep the validation rules, setup the store, and not change my continuous integration or continuous deployment plans at the moment.

I might minimally modify some of those copied seed files, though I would rather have left the seeding of the database alone and modified the Store configuration with other purpose built Rake tasks.

It would be nice if the extension could somehow nudge / warn the administrators of the Solidus install that their stock locations need addresses, otherwise shipping calculation won't be possible.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants