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

feat/BILL-CPC-933_Implement_Button_To_Calculate_Bill_With_Tax #572

Merged

Conversation

HennaCH
Copy link
Collaborator

@HennaCH HennaCH commented Oct 22, 2023

JIRA: https://champlainsaintlambert.atlassian.net/browse/CPC-933

Context:

We need to see the amount to pay including taxes

Changes

  • Added taxedAmount field in BillResponseDTO and Bill
  • Added tax calculation in BillServiceImpl and BillServiceClient
  • I modified the BillResponseDTP in the tests so that they include the taxedAmount
  • Made the button to display the taxed amount

Before and After UI (Required for UI-impacting PRs)

Before
Screenshot (1130)

After
Screenshot (1144)


Screenshot (1145)

Dev notes (Optional)

  • I am not sure if my changes in the test sections are good

@github-actions
Copy link

github-actions bot commented Oct 22, 2023

Qodana for JVM

374 new problems were found

Inspection name Severity Problems
Unused import 🔶 Warning 103
Redundant character escape 🔶 Warning 85
Dangling Javadoc comment 🔶 Warning 15
Invalid YAML configuration 🔶 Warning 11
Integer multiplication or shift implicitly cast to 'long' 🔶 Warning 10
AutoCloseable used without 'try'-with-resources 🔶 Warning 6
Optional.get() is called without isPresent() check 🔶 Warning 6
Default annotation parameter value 🔶 Warning 5
Field can be local 🔶 Warning 5
Redundant local variable 🔶 Warning 5
Nullability and data flow problems 🔶 Warning 4
Deprecated API usage 🔶 Warning 4
Link specified as plain text 🔶 Warning 4
@NotNull/@Nullable problems 🔶 Warning 4
Spring Data repository method parameters errors 🔶 Warning 4
Unused assignment 🔶 Warning 4
Possibly blocking call in non-blocking context 🔶 Warning 3
Call to 'printStackTrace()' 🔶 Warning 3
'Optional' used as field or parameter type 🔶 Warning 2
Unused publisher 🔶 Warning 2
Redundant type cast 🔶 Warning 2
Redundant 'close()' 🔶 Warning 2
Calling 'subscribe' in "reactive" methods 🔶 Warning 1
Field may be 'final' 🔶 Warning 1
Lombok @Getter may be used 🔶 Warning 1
Mismatch in @PathVariable declarations and usages 🔶 Warning 1
Begin or end anchor in unexpected position 🔶 Warning 1
'size() == 0' can be replaced with 'isEmpty()' 🔶 Warning 1
Unnecessary call to 'toString()' 🔶 Warning 1
Commented out code ◽️ Notice 35
Duplicated code fragment ◽️ Notice 19
Non recommended 'field' injections ◽️ Notice 19
Constant values ◽️ Notice 2
Mismatch in @PathVariable declarations and usages ◽️ Notice 1
Throw statement in Reactive operator ◽️ Notice 1
Regular expression can be simplified ◽️ Notice 1

☁️ View the detailed Qodana report

Contact Qodana team

Contact us at [email protected]

@HennaCH HennaCH changed the title I did the calculation for the taxed amount and used a button to displ… feat/BILL-CPC-933_Implement_Button_To_Calculate_Bill_With_Tax Oct 22, 2023
drallo22
drallo22 previously approved these changes Oct 22, 2023
@@ -42,6 +42,8 @@ <h5>Due Date: <strong>{{$ctrl.bills.dueDate}}</strong></h5>
<hr>
<br>
<h5>Final Amount Due: {{$ctrl.bills.amount}}</h5>
<h5 ng-if="show">Final Amount Including Taxes: {{$ctrl.bills.taxedAmount}}</h5>
<button class="newBtn btn btn-primary" ng-click="show = !show"> {{show? "Hide amount including taxes" : "Show amount including taxes"}}</button>
Copy link
Collaborator

Choose a reason for hiding this comment

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

You also could have done this by crating a style class that is hidden, and then when the button is clicked the style class is then shown! Just a small change, either way works well!

Copy link
Collaborator Author

@HennaCH HennaCH Oct 22, 2023

Choose a reason for hiding this comment

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

Ok, I will try your suggestion !

ebond07
ebond07 previously approved these changes Oct 22, 2023
Copy link
Collaborator

@ebond07 ebond07 left a comment

Choose a reason for hiding this comment

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

Good job adding in a tax for the bills.

Comment on lines 15 to 16
//Mono<BillResponseDTO> getTaxedAmountByBillIdAndBillAmount(String billId, Double amount);

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this left here so that eventually it will be implemented in the service impl? If not I don't think it would be a bad idea to get rid of commented out lines that we aren't using.

@HennaCH HennaCH dismissed stale reviews from ebond07 and drallo22 via 3ba0ad7 October 24, 2023 01:33
@HennaCH HennaCH merged commit e8e258b into main Oct 24, 2023
3 checks passed
@HennaCH HennaCH deleted the feat/BILL-CPC-933_Implement_Button_To_Calculate_Bill_With_Tax branch October 24, 2023 02:12
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.

3 participants