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.

Sign up for GitHub

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

Jump to bottom

[#161995] [Shared dev] Duration based pricing - (5) Duration discounts applied to user reservation #3872

Merged
merged 47 commits into from
Dec 8, 2023
Merged

[#161995] [Shared dev] Duration based pricing - (5) Duration discounts applied to user reservation #3872

merged 47 commits into from
Dec 8, 2023

Conversation

LeticiaErrandonea
Copy link
Collaborator

@LeticiaErrandonea LeticiaErrandonea commented Dec 1, 2023 •
edited
Loading

Release Notes

  • Update stepped billing calculation so it uses the step rate for internal price groups.
  • Require that either none or all steps are filled.
  • Show calculation for each step in the Price policy index (only internal groups).

Screenshot

image image

@LeticiaErrandonea LeticiaErrandonea force-pushed the 161995-duration-based-for-internal-groups branch from 279806c to 4a72f83 Compare December 4, 2023 18:32
giladshanan
giladshanan requested changes Dec 5, 2023
View reviewed changes
Copy link
Collaborator

@giladshanan giladshanan left a comment

Choose a reason for hiding this comment

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

This is really close - just a few small asjustments

app/models/duration_rate.rb Outdated Show resolved Hide resolved
app/assets/javascripts/price_policy.js Outdated Show resolved Hide resolved
spec/system/admin/instrument_price_policies_controller_spec.rb Outdated Show resolved Hide resolved
giladshanan
giladshanan reviewed Dec 5, 2023
View reviewed changes
app/models/duration_rate.rb Outdated Show resolved Hide resolved
app/models/price_policy.rb Show resolved Hide resolved
@LeticiaErrandonea LeticiaErrandonea force-pushed the 161995-duration-based-for-internal-groups branch from 6c9afa1 to 351dd9a Compare December 6, 2023 19:00
@LeticiaErrandonea LeticiaErrandonea marked this pull request as ready for review December 6, 2023 19:49
@LeticiaErrandonea LeticiaErrandonea requested a review from a team as a code owner December 6, 2023 19:49
giladshanan
giladshanan reviewed Dec 6, 2023
View reviewed changes
app/models/order_detail.rb Outdated Show resolved Hide resolved
db/migrate/20231206192754_delete_all_duration_rates.rb
class DurationRate < ApplicationRecord; end

def change
DurationRate.delete_all
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should make a note to do some manual testing around this before releasing

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You mean like checking that price policies aren't broken? Any other cases you can think of?

Copy link
Collaborator

Choose a reason for hiding this comment

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

If we have any duration rate records on production, we should check if any orders have been placed against them. I don't think anyone is using this yet but I just want to be safe

@LeticiaErrandonea
Update validations
1e8241a 
- DurationRate
    - Rate always present
    - Subsidy required for internal (not base)
    - subsidy <= rate
  - Price policy
    - Each group can have either none or 3 duration rates
@LeticiaErrandonea
Update calculations and its specs
a796a5a
@LeticiaErrandonea
Show calculation
bbb2d9e
@LeticiaErrandonea
Update rate hidden field when step is changed
574d37f
@LeticiaErrandonea
Add specs for calculation in index
d0b4590
@LeticiaErrandonea
Update specs for new validations
e6d7973
@LeticiaErrandonea
Fix specs
d58f7ae
@LeticiaErrandonea
Update message error
7509a52
@LeticiaErrandonea
Add requires_subsidy? method
24c7e73
@LeticiaErrandonea
Move up rate field init
269261c
@LeticiaErrandonea
WIP: Manage order details spec
512ced9
@LeticiaErrandonea
Finish Manage Order detail spec
6955b30
@LeticiaErrandonea
Remove field in factory that caused errors (and solved nothing)
0664933
@LeticiaErrandonea
Add js tag
f387ee3
@LeticiaErrandonea
Fix selector
31caf97
@LeticiaErrandonea
Tab in order to blur and get fields populated
3764f45
@LeticiaErrandonea
Update OrderDetail#assign_actual_price so default value is set
73b4ffe
@LeticiaErrandonea
Fix expected values
1d958e1
@LeticiaErrandonea
Add migration to delete previous DurationRate
7e57c83
@LeticiaErrandonea
Add custom message for missing subsidy
61e34f0
@LeticiaErrandonea
Add frozen_string_literal: true
6cffee5
@LeticiaErrandonea LeticiaErrandonea force-pushed the 161995-duration-based-for-internal-groups branch from 0e6ba7c to 6cffee5 Compare December 7, 2023 15:20
@dependabot @giladshanan
Bump teaspoon from 1.2.2 to 1.4.0 (#3665)
106a27b 
* Bump teaspoon from 1.2.2 to 1.4.0

Bumps [teaspoon](https://github.com/jejacks0n/teaspoon) from 1.2.2 to 1.4.0.
- [Release notes](https://github.com/jejacks0n/teaspoon/releases)
- [Changelog](https://github.com/jejacks0n/teaspoon/blob/master/CHANGELOG.md)
- [Commits](jejacks0n/teaspoon@v1.2.2...v1.4.0)

---
updated-dependencies:
- dependency-name: teaspoon
  dependency-type: indirect
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

* Bump teaspoon from 1.2.2 to 1.4.0

Bumps [teaspoon](https://github.com/jejacks0n/teaspoon) from 1.2.2 to 1.4.0.
- [Release notes](https://github.com/jejacks0n/teaspoon/releases)
- [Changelog](https://github.com/jejacks0n/teaspoon/blob/master/CHANGELOG.md)
- [Commits](jejacks0n/teaspoon@v1.2.2...v1.4.0)

---
updated-dependencies:
- dependency-name: teaspoon
  dependency-type: indirect
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

* Use an older image tag for selenium

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Gilad Shanan <[email protected]>
LeticiaErrandonea and others added 25 commits December 7, 2023 15:45
@LeticiaErrandonea @giladshanan
Update validations
8025f66 
- DurationRate
    - Rate always present
    - Subsidy required for internal (not base)
    - subsidy <= rate
  - Price policy
    - Each group can have either none or 3 duration rates
@LeticiaErrandonea @giladshanan
Update calculations and its specs
e008bda
@LeticiaErrandonea @giladshanan
Show calculation
29aaafb
@LeticiaErrandonea @giladshanan
Update rate hidden field when step is changed
9773b82
@LeticiaErrandonea @giladshanan
Add specs for calculation in index
99e48c8
@LeticiaErrandonea @giladshanan
Update specs for new validations
1f54d33
@LeticiaErrandonea @giladshanan
Fix specs
7ea0c05
@LeticiaErrandonea @giladshanan
Update message error
7ab6e43
@LeticiaErrandonea @giladshanan
Add requires_subsidy? method
7b1c930
@LeticiaErrandonea @giladshanan
Move up rate field init
a8b26f9
@LeticiaErrandonea @giladshanan
WIP: Manage order details spec
92247aa
@LeticiaErrandonea @giladshanan
Finish Manage Order detail spec
c34e874
@LeticiaErrandonea @giladshanan
Remove field in factory that caused errors (and solved nothing)
3f3b693
@LeticiaErrandonea @giladshanan
Add js tag
992aa9b
@LeticiaErrandonea @giladshanan
Fix selector
ed5fc6b
@LeticiaErrandonea @giladshanan
Tab in order to blur and get fields populated
d20f453
@LeticiaErrandonea @giladshanan
Update OrderDetail#assign_actual_price so default value is set
982b6c7
@LeticiaErrandonea @giladshanan
Fix expected values
c020937
@LeticiaErrandonea @giladshanan
Add migration to delete previous DurationRate
b480517
@LeticiaErrandonea @giladshanan
Add custom message for missing subsidy
24b85d4
@LeticiaErrandonea @giladshanan
Add frozen_string_literal: true
351122b
@giladshanan
Spec cleanup
1acb5c3
@giladshanan
Bump selenium-webdriver to see if it makes teaspoon happy
82c9b8e
@giladshanan
Merge branch '161995-duration-based-for-internal-groups' of https://g…
8003a18 
…ithub.com/tablexi/nucore-open into 161995-duration-based-for-internal-groups
@giladshanan
Add code comment
30e2419
giladshanan
giladshanan approved these changes Dec 7, 2023
View reviewed changes
Copy link
Collaborator

@giladshanan giladshanan left a comment

Choose a reason for hiding this comment

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

Pending CI 🤞🏼

@LeticiaErrandonea LeticiaErrandonea merged commit 84988c0 into master Dec 8, 2023
3 checks passed
@LeticiaErrandonea LeticiaErrandonea deleted the 161995-duration-based-for-internal-groups branch December 8, 2023 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@giladshanan giladshanan giladshanan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@LeticiaErrandonea @giladshanan