Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

V2 #14

Open
wants to merge 54 commits into
base: v1.4
Choose a base branch
from
Open

V2 #14

wants to merge 54 commits into from

Conversation

papayaah
Copy link

Hi @bryanmtl !

here are the changes that I moved out of the glossier code.

@bryanmtl
Copy link

Thanks @mickeyren. Are there any specs that go along with these changes?

@papayaah
Copy link
Author

@bryanmtl i can imagine writing a spec for the subscription renewal job and checking for the cancellation fields.

Can you grant me admin settings? I like to put this up on travis.org.

@papayaah
Copy link
Author

@bryanmtl there's a bunch of errors coming out of the spec suite, even on the master branch but I'm more than halfway through. Just a little setback as I get myself acquainted with state_machines

@bryanmtl
Copy link

Hey @mickeyren - I've enabled Travis on this repo. Let me know if you have access now.

@papayaah
Copy link
Author

@bryanmtl getting there. I actually had to clean up a bunch of issues on the admin pages, improving the UI along the way. Now has tabs!

image

So far I've been fixing failing specs and admin pages issues so I still yet to write the specs for the new code that were introduced in this PR.

@papayaah
Copy link
Author

papayaah commented Nov 19, 2016

bryanmtl specs are green, well sort of. and i've put in a simple spec for the subscription renewal.

There is this one last failing spec that happens only on Travis. It's failing to reach the Stripe gateway.

image

this is the spec:

before(:each) do
    Stripe.api_key = "sk_test_jTiNI1BxjFxBr4TUqdHefc1f"

    user = create(:user)
    setup_subscriptions_for user
    sign_in_as! user

    my_account = MyAccount::Page.new
    @credit_card = my_account.any_subscription.edit.payment_details
  end

  scenario "displays the current payment info" do
    expect(@credit_card.current_payment_info).to_not be_nil
  end

  scenario "can update payment info" do
    @credit_card.number = "4242424242424242"
    @credit_card.name = "John Doe"
    @credit_card.expiry = "06/20"
    @credit_card.code = "123"

    @credit_card.submit

    expect(@credit_card.current_payment_info).to have_text("4242")
    expect(@credit_card.current_payment_info).to have_text("John Doe")
    expect(@credit_card.current_payment_info).to have_text("6/2020")
  end

it's failing on the last one.

cc @aamyot

@bryanmtl
Copy link

@mickeyren Do you need test Stripe credentials to get this passing? Does that spec work for you locally?

papayaah and others added 9 commits December 1, 2016 21:02
remove tooltip - non existent anymore.
* v2: (37 commits)
  clean up and fix a few locale settings
  fix editing of item quantity and deleting
  show sku of the variant
  fix product selection remove tooltip - non existent anymore.
  subscription renewal spec
  using poltergeist as the default driver for capybara seem to be breaking the specs
  provide missing en setting that's breaking the spec also fix state - when resuming at a later period, state should remain at paused
  added capybara screenshot to test remove duplicate subscription links
  use poltergeist for capybara
  do not unpause if resume is in the future
  no need for npm install
  install phantom js before travis runs tests
  rename to solidus_subscriptions
  correct way to add menu to the admin sidebar also added submenu for the subscriptions tab
  make sure adjust sku and failed renewals only show up under the subscription listing page
  tabify the admin edit subscription page to be consistent with v1.4
  fix subscription state column translation
  fix subscription state column translation
  remove portrait_small_url, this doesn't belong here and should be in the app using this
  refactor some specs, by providing some convenient method to pull next_shipment_dates from the subscription
  ...
fixes adjust sku service for subscriptions
@papayaah
Copy link
Author

@bryanmtl @aamyot

would you guys mind looking over the strange error?

$ bundle exec rake test_app
Expected string default value for '--rc'; got false (boolean)
rake aborted!
NoMethodError: undefined method `last_comment' for #<Rake::Application:0x000000019a4bf0>

I don't find any trace of last_comment anywhere and the spec suite passes just fine for me locally.

papayaah and others added 12 commits January 19, 2017 21:57
…ulls

fix the last order in the admin listing page so it correctly shows the actual last order
ensure a working versionr Rakefile
And this is causing issues with a particular test. In order to avoid
that I reached out to my trusty VCR and threw it at the problematic
spec. I've noticed that other tests are doing HTTP too but I don't want
to do a full pass at this point.
…oller

correct syntax for the controller callbacks
* surface the currency used in the order to easily identify where the order came from

* there has been a failure with the renewal for international orders, the shipping rates
are being calculated for US addresses by default, if the currency is not set in the order.

Setting the currency for new orders generated ensures the right shipping methods are pulled
to match the customer's address

* a little comment to understand why the update of email comes after the creation of the order

* specs to ensure the currency and store id is set from the subscription's last completed order

* refactor into a callback, for clarity and less comments
* use subscriptions last completed orders currency when adding items to subscription

* moves currency logic to subscription model
* we don't do weeks so let's default to 1 month

* correct syntax to build a subscription from creating a new one from an order

should have also been saving the order's email

* swap HABTM with has_many as HABTM has many limitations

changing the default interval to months as we don't do weeks

* html fixes for the page

* the model representation for the order subscription join table

* changes to the spec to reflect the new interval type
* need to exclude those fields that are not present in the subscription address table

* support for UK addresses

* the same here - non-existing fields in the subscription address should be excluded
* initial work on having a page to display subscriptions stuck in the renewal process

* added filter by status on the subscriptions listing page

start showing the renewing subscription. these are subscription(state) that were stuck therefore they belong to this list

* decided to just add a filter and add the renewing state in the failure page

* as required by travis when creating the dummy app:
"deface is required to run solidus_auth_devise with solidus versions < 2.5.
Please add deface to your Gemfile"

also time to bump up the gem version. this is now using a state machine
to set the current state of the subscription
* stop showing cancelled subscriptions and sort by recent subscription renewal

* clearer labels

* this should fix the sequence of orders appearing in the list

* spec to test that filtering by subscription status works

* initial work on the spec for testing the failures listing

* specs to test that only subscriptions that failed renewing are shown, active and cancelled are not
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants