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-905_Implement_Pagination_System_For_BillHistory_Page #599

Conversation

dazhuox
Copy link
Collaborator

@dazhuox dazhuox commented Oct 24, 2023

JIRA: https://champlainsaintlambert.atlassian.net/browse/CPC-905?atlOrigin=eyJpIjoiNmI1ODdhOWRiM2M4NDJkMTkwZjdhN2Q4NjgyMjg0NjMiLCJwIjoiaiJ9

Context:

This ticket implements the pagination system for the Bill History page (getAllBillsByPage).

Changes

  • There are buttons now to help the user navigate from one set of list (page) of bills to another.
  • The url of the page is changed to the new pagination method.
  • The url accepts params that allow the selection of how many items users want to see per page.

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

Before implementation:
image

After implementation:
image

Dev notes (Optional)

  • Specific technical changes that should be noted

Linked pull requests (Optional)

  • pull request link

Copy link
Collaborator

@drallo22 drallo22 left a comment

Choose a reason for hiding this comment

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

Very useful especially if we have a lot of bills, good job!

// Add query parameters conditionally if they are not null or empty
if (billId != null && !billId.isEmpty()) {
builder.queryParam("billId", billId);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could have been done using switch or if-else statements but this works fine as well!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I didn't think about that, it could definitely work. My usual preference are if statements so I'll stick with that

Copy link
Collaborator

@swafit swafit Oct 24, 2023

Choose a reason for hiding this comment

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

Could have been done using switch or if-else statements but this works fine as well!

In this case an if-else is better since you might have more than one query param that is not provided.

@dazhuox dazhuox changed the title Feat/bill cpc 905 implement pagination system for bill history page feat/BILL-CPC-905_Implement_Pagination_System_For_BillHistory_Page Oct 24, 2023
@github-actions
Copy link

github-actions bot commented Oct 24, 2023

Qodana for JVM

419 new problems were found

Inspection name Severity Problems
Unused import 🔶 Warning 129
Redundant character escape 🔶 Warning 85
Dangling Javadoc comment 🔶 Warning 15
Integer multiplication or shift implicitly cast to 'long' 🔶 Warning 12
Invalid YAML configuration 🔶 Warning 9
Field can be local 🔶 Warning 7
AutoCloseable used without 'try'-with-resources 🔶 Warning 6
Redundant 'Collection.addAll()' call 🔶 Warning 6
Optional.get() is called without isPresent() check 🔶 Warning 6
Redundant local variable 🔶 Warning 6
Default annotation parameter value 🔶 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
Field may be 'final' 🔶 Warning 3
Lombok @Getter may be used 🔶 Warning 3
Call to 'printStackTrace()' 🔶 Warning 3
Non-final field in 'enum' 🔶 Warning 2
'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
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
Unchecked warning 🔶 Warning 1
Unnecessary call to 'toString()' 🔶 Warning 1
Commented out code ◽️ Notice 36
Duplicated code fragment ◽️ Notice 21
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]

@dazhuox dazhuox merged commit 584ab41 into main Oct 24, 2023
3 checks passed
@dazhuox dazhuox deleted the feat/BILL-CPC-905_Implement_Pagination_System_for_BillHistory_page branch October 24, 2023 17:25
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