-
-
Notifications
You must be signed in to change notification settings - Fork 729
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
3283 [Enterprise Fee Summary] Fix values when calculator is order-based #3337
Merged
sauloperez
merged 12 commits into
openfoodfoundation:master
from
kristinalim:feature-enterprise_fees_report-order_based_calculator_bug
Feb 12, 2019
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
06ba518
Add flat rate calculator traits to related factories
kristinalim 0212454
Refactor SQL result summarizer for report
kristinalim e550be9
Add test for using order-based exchange fees
kristinalim 8f65bee
Clarify method name in report scope
kristinalim c4801aa
Fix Fee Calc on Transfer Through and Tax Category
kristinalim 7a76355
Separate summarizing of SQL data by source
kristinalim 373e3e1
Simplify name of SQL data summarizer class
kristinalim b9c144b
Prepare to change AssociatedEnterpriseFee to module
kristinalim 664943a
Include UsingEnterpriseFee instead of inheriting
kristinalim bc95560
Add descriptions for data representation classes
kristinalim ddc788e
Move data representation translations to class
kristinalim 5e8a336
Initialize data representation classes with data
kristinalim File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
27 changes: 27 additions & 0 deletions
27
...s/order_management/reports/enterprise_fee_summary/data_representations/coordinator_fee.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
# This module provides EnterpriseFeeSummary::Scope DB result to report mappings for coordinator fees | ||
# in an order cycle. | ||
|
||
module OrderManagement | ||
module Reports | ||
module EnterpriseFeeSummary | ||
module DataRepresentations | ||
class CoordinatorFee | ||
include UsingEnterpriseFee | ||
|
||
def fee_calculated_on_transfer_through_name | ||
i18n_translate("fee_calculated_on_transfer_through_all") | ||
end | ||
|
||
def tax_category_name | ||
return data["tax_category_name"] if data["tax_category_name"].present? | ||
i18n_translate("tax_category_various") if inherits_tax_category? | ||
end | ||
|
||
def inherits_tax_category? | ||
data["enterprise_fee_inherits_tax_category"] == "t" | ||
end | ||
end | ||
end | ||
end | ||
end | ||
end |
23 changes: 23 additions & 0 deletions
23
...rder_management/reports/enterprise_fee_summary/data_representations/exchange_order_fee.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
# This module provides EnterpriseFeeSummary::Scope DB result to report mappings for exchange fees | ||
# that use order-based calculators. | ||
|
||
module OrderManagement | ||
module Reports | ||
module EnterpriseFeeSummary | ||
module DataRepresentations | ||
class ExchangeOrderFee | ||
include UsingEnterpriseFee | ||
|
||
def fee_calculated_on_transfer_through_name | ||
i18n_translate("fee_calculated_on_transfer_through_entire_orders", | ||
distributor: data["adjustment_source_distributor_name"]) | ||
end | ||
|
||
def tax_category_name | ||
data["tax_category_name"] || i18n_translate("tax_category_various") | ||
end | ||
end | ||
end | ||
end | ||
end | ||
end |
22 changes: 22 additions & 0 deletions
22
...nt/reports/enterprise_fee_summary/data_representations/incoming_exchange_line_item_fee.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
# This module provides EnterpriseFeeSummary::Scope DB result to report mappings for incoming | ||
# exchange fees that use line item -based calculators. | ||
|
||
module OrderManagement | ||
module Reports | ||
module EnterpriseFeeSummary | ||
module DataRepresentations | ||
class IncomingExchangeLineItemFee | ||
include UsingEnterpriseFee | ||
|
||
def fee_calculated_on_transfer_through_name | ||
data["incoming_exchange_enterprise_name"] | ||
end | ||
|
||
def tax_category_name | ||
data["tax_category_name"] || data["product_tax_category_name"] | ||
end | ||
end | ||
end | ||
end | ||
end | ||
end |
22 changes: 22 additions & 0 deletions
22
...nt/reports/enterprise_fee_summary/data_representations/outgoing_exchange_line_item_fee.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
# This module provides EnterpriseFeeSummary::Scope DB result to report mappings for outgoing | ||
# exchange fees that use line item -based calculators. | ||
|
||
module OrderManagement | ||
module Reports | ||
module EnterpriseFeeSummary | ||
module DataRepresentations | ||
class OutgoingExchangeLineItemFee | ||
include UsingEnterpriseFee | ||
|
||
def fee_calculated_on_transfer_through_name | ||
data["outgoing_exchange_enterprise_name"] | ||
end | ||
|
||
def tax_category_name | ||
data["tax_category_name"] || data["product_tax_category_name"] | ||
end | ||
end | ||
end | ||
end | ||
end | ||
end |
38 changes: 38 additions & 0 deletions
38
...rder_management/reports/enterprise_fee_summary/data_representations/payment_method_fee.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
# This module provides EnterpriseFeeSummary::Scope DB result to report mappings for payment method | ||
# fees. | ||
|
||
module OrderManagement | ||
module Reports | ||
module EnterpriseFeeSummary | ||
module DataRepresentations | ||
class PaymentMethodFee | ||
include WithI18n | ||
|
||
attr_reader :data | ||
|
||
def initialize(data) | ||
@data = data | ||
end | ||
|
||
def fee_type | ||
i18n_translate("fee_type.payment_method") | ||
end | ||
|
||
def enterprise_name | ||
data["hub_name"] | ||
end | ||
|
||
def fee_name | ||
data["payment_method_name"] | ||
end | ||
|
||
def fee_placement; end | ||
|
||
def fee_calculated_on_transfer_through_name; end | ||
|
||
def tax_category_name; end | ||
end | ||
end | ||
end | ||
end | ||
end |
40 changes: 40 additions & 0 deletions
40
...der_management/reports/enterprise_fee_summary/data_representations/shipping_method_fee.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
# This module provides EnterpriseFeeSummary::Scope DB result to report mappings for shipping method | ||
# fees. | ||
|
||
module OrderManagement | ||
module Reports | ||
module EnterpriseFeeSummary | ||
module DataRepresentations | ||
class ShippingMethodFee | ||
include WithI18n | ||
|
||
attr_reader :data | ||
|
||
def initialize(data) | ||
@data = data | ||
end | ||
|
||
def fee_type | ||
i18n_translate("fee_type.shipping_method") | ||
end | ||
|
||
def enterprise_name | ||
data["hub_name"] | ||
end | ||
|
||
def fee_name | ||
data["shipping_method_name"] | ||
end | ||
|
||
def fee_placement; end | ||
|
||
def fee_calculated_on_transfer_through_name; end | ||
|
||
def tax_category_name | ||
i18n_translate("tax_category_name.shipping_instance_rate") | ||
end | ||
end | ||
end | ||
end | ||
end | ||
end |
40 changes: 40 additions & 0 deletions
40
...er_management/reports/enterprise_fee_summary/data_representations/using_enterprise_fee.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
# Different EnterpriseFeeSummary::Scope DB result attributes are checked when dealing with | ||
# enterprise fees that are attached to an order cycle in different ways. | ||
# | ||
# This module provides DB result to report mappings that are common among all rows for enterprise | ||
# fees. These mappings are not complete and should be supplemented with mappings that are specific | ||
# to the way that the enterprise fee is attached to the order cycle. | ||
|
||
module OrderManagement | ||
module Reports | ||
module EnterpriseFeeSummary | ||
module DataRepresentations | ||
module UsingEnterpriseFee | ||
include WithI18n | ||
|
||
attr_reader :data | ||
|
||
def initialize(data) | ||
@data = data | ||
end | ||
|
||
def fee_type | ||
data["fee_type"].try(:capitalize) | ||
end | ||
|
||
def enterprise_name | ||
data["enterprise_name"] | ||
end | ||
|
||
def fee_name | ||
data["fee_name"] | ||
end | ||
|
||
def fee_placement | ||
i18n_translate("fee_placements.#{data['placement_enterprise_role']}") | ||
end | ||
end | ||
end | ||
end | ||
end | ||
end |
15 changes: 15 additions & 0 deletions
15
...ervices/order_management/reports/enterprise_fee_summary/data_representations/with_i18n.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
module OrderManagement | ||
module Reports | ||
module EnterpriseFeeSummary | ||
module DataRepresentations | ||
module WithI18n | ||
private | ||
|
||
def i18n_translate(translation_key, options = {}) | ||
I18n.t("order_management.reports.enterprise_fee_summary.#{translation_key}", options) | ||
end | ||
end | ||
end | ||
end | ||
end | ||
end |
87 changes: 0 additions & 87 deletions
87
...s/order_management/reports/enterprise_fee_summary/enterprise_fee_type_total_summarizer.rb
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this namespace provide other than making the nesting level pretty wide? These model classes won't clash with any other within the
EnterpriseFeeSummary
namespace, so I'd just removeDataRepresentations
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disagree about removing
DataRepresentations
, @sauloperez.The class ends up more deeply nested, but
DataRepresentations
I think makes it easier to find/ignore these classes and gives you an idea about their purpose.If we reduce the nesting, I would want to rename the individual classes to reflect that they map DB query row attributes to report row attributes. For example, append "Representation" or "Presenter" (both terms actually from review of @mkllnk). Compared to this, I prefer nesting.
Let me know if you are okay with keeping this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this way of using namespaces to organize your code! #java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to me, it's pretty obvious already. The class name is a noun named after a domain concept. What else could it be? But if you guys prefer it, I don't mind.