Skip to content
This repository has been archived by the owner on Jul 24, 2020. It is now read-only.

[1449] Fix cart availability validation [master] #1452

Merged
merged 1 commit into from
Feb 9, 2016

Conversation

orenyk
Copy link
Contributor

@orenyk orenyk commented Jan 24, 2016

Resolves #1449 on master

  • replace reserved_in_date_range with overlaps_with_date_range that doesn't take status into account and add active where necessary
  • add specs for cart availability validation
  • fix specs for reservation availability validation

@AlanLiu96
Copy link
Contributor

As something to consider, should we also disable the add to cart button on the equipment model pages similar to how we disable the add to cart button on the catalog if the item isn't reservable between the dates in the cart?

@orenyk
Copy link
Contributor Author

orenyk commented Jan 31, 2016

We could, it would improve our UX, but I'd add that as a separate issue. I'll open one up.

@orenyk
Copy link
Contributor Author

orenyk commented Jan 31, 2016

Also, did you review this? If not, could you 😛?

@esoterik
Copy link
Collaborator

esoterik commented Feb 2, 2016

Should we also add tests for different start/end date overlaps in the cart spec?

@orenyk orenyk force-pushed the 1449_availability_bug branch from 43c49cc to 461764d Compare February 2, 2016 05:14
@orenyk
Copy link
Contributor Author

orenyk commented Feb 2, 2016

I added tests for overlapping reservations in the cart specs, ready for re-review!

@orenyk
Copy link
Contributor Author

orenyk commented Feb 4, 2016

Turns out our availability calculations are still broken (see here), so I'm blocking this on the fix for v5.5 with that fix being added to this branch along with tests.

@orenyk
Copy link
Contributor Author

orenyk commented Feb 7, 2016

Nvm, I included tests for #1479 in #1482, so this is good to go as well.

@@ -136,4 +136,47 @@
expect { @cart.fix_items }.to change { @cart.items.length }.by(-1)
end
end

describe 'check_availability' do
before do
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is super nitpicky but can we specify :each here for consistency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure no problem

@zeffman
Copy link
Contributor

zeffman commented Feb 9, 2016

Looks good

Resolves #1449
- replace reserved_in_date_range with overlaps_with_date_range that
  doesn't take status into account and add active where necessary
- add specs for cart availability validation
- fix specs for reservation availability validation
@orenyk orenyk force-pushed the 1449_availability_bug branch from 461764d to 0e29249 Compare February 9, 2016 02:03
@orenyk
Copy link
Contributor Author

orenyk commented Feb 9, 2016

Addressed the one in-line comment, merging following a green build.

orenyk added a commit that referenced this pull request Feb 9, 2016
[1449] Fix cart availability validation
@orenyk orenyk merged commit c96e993 into master Feb 9, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Availability validation not working? [v5.5]
4 participants