Skip to content

[BE] PR 2/3: Point History Retrieval Logic (#216)#1118

Merged
ivilarop merged 25 commits intodevelopfrom
feature/190-2/point-history-retrival-logic
Feb 25, 2026
Merged

[BE] PR 2/3: Point History Retrieval Logic (#216)#1118
ivilarop merged 25 commits intodevelopfrom
feature/190-2/point-history-retrival-logic

Conversation

@Lucy-SD
Copy link
Collaborator

@Lucy-SD Lucy-SD commented Feb 24, 2026

Feature: Implementation of points history and accumulation logic

closes sub-issue: #IT-Academy-BCN/ita-challenges#216

📝 Description

Implementation of the core business logic to retrieve a student's point history. This service processes raw database entries into a structured format suitable for growth charts.

✅ Acceptance Criteria

  • Logic calculates totalPoints as the sum of all historical entries.
  • Entries are sorted in decreasing order by date.
  • Dates are returned in ISO 8601 format.

🔧 Technical Details

  • DTOs: PointsHistoryResponseDto (contains totalPoints and a list of PointHistoryEntryDto).
  • Service Logic: UserScoreServiceImp consumes the Flux<UserScoreDocument> from the repository, using collectList() operator to process the data reactively. In a single transformation step, it calculates the arithmetic sum of totalPoints and maps the documents to the PointHistoryEntryDto, returning a Mono<PointsHistoryResponseDto>.
  • Exceptions: Proper handling of UserNotFound using the common.exception package.

💰 Value Provided

Transforms raw data into "Progress Information," allowing students to visualize their growth over time.

🔓Unlocks

Unblocks the final API Controller: sub-issue #217; and provides the data structure for Frontend graph components.

🔗 Dependencies

Chained to #215 - (PR #1107)
Closes #IT-Academy-BCN/ita-challenges#216

🧪 Test Implementations

  • Unit Tests: Mocking the Repository to verify the sum calculation and chronological sorting logic in the Service.

🔄 Acceptance Criteria (DoD)

  • No hardcoded data in the Service.
  • Code coverage for logic > 80%.
  • PR description links to the previous shared PR.

👨‍💻 Technical Debts

  • totalPoints is calculated on-the-fly. For high-volume users, an "Aggregated Totals" table might be needed in the future.
  • Username Enrichment: Currently, the username is retrieved from the history. If the history is empty, the field returns an empty string. It is recommended to extend IChallengeJwtFacade to extract the username directly from the JWT in future iterations.

@Lucy-SD
Copy link
Collaborator Author

Lucy-SD commented Feb 24, 2026

IMPORTANT INFO

The original PR was closed by mistake; I've attached the link where you can find the corrections and comments from colleagues.

#1109

All Tests Pass = )

image

class PointsHistoryResponseDtoTest {

@Test
void testDtoStructure() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Issues commented in orginal PR:

#1109 (comment)

[ivilarop]:

This test only checks Lombok-generated builder/getters. It increases coverage but doesn’t protect behavior. If we want to validate the API contract, a Jackson serialization test (e.g., date mapping) would provide real value.

Answer:
[Lucy-SD]:
Done (in separated class).

Copy link
Collaborator

Choose a reason for hiding this comment

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

I understand that testDtoStructure is for coverage/Sonar, but now that we have @JsonTest that validates the actual contract, could we remove this test and the class?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Apparently it could be done, I'll remove it and if for some reason Sonar fails again I'll add it back.

@Lucy-SD
Copy link
Collaborator Author

Lucy-SD commented Feb 24, 2026

Issues addressed in original PR:

#1109 (comment)

[ivilarop]:
I would remove the GamificationMapper for now. The mapping is very simple (2 fields + null handling) and is only used in this service, so the extra class doesn't reduce complexity; it just adds a navigation jump and clutter. Furthermore, the name GamificationMapper is too generic and breaks naming/semantics: it looks like a mapper for the entire module, but in reality, it only maps point history entries (UserScoreDocument -> PointHistoryEntryDto).

Proposal: move the mapping to a private method in PointsServiceImpl (or to the stream itself using a lambda function), and if future reuse or further transformations arise (ISO date format, derived fields, etc.), then extract a PointsHistoryMapper with a specific name.

Answer:
[Lucy-SD]:
The idea behind having a separate mapper was to be able to reuse it for ranking and future implementations. At this stage of development, I understand that it's not necessary. Removed.

@Lucy-SD
Copy link
Collaborator Author

Lucy-SD commented Feb 24, 2026

All tests still passing after updates = )

image

Copy link
Collaborator

@carlosPc1987 carlosPc1987 left a comment

Choose a reason for hiding this comment

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

This aligns well with the persistence layer we have in #1108:
UserScoreDocument with pointsEarned, UserScoreRepository.findByUserIdOrderByCreatedAtDesc,
and the unique index on (user_id, challenge_id).
The removal of distinct() is consistent with that.
When we merge both PRs we’ll have recordPoints (from #1108) and getUserPointsHistory (this PR) under the same UserScoreService naming.
No further comments from my side.
Good job Lucy!

Copy link
Collaborator

@pantalois pantalois left a comment

Choose a reason for hiding this comment

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

Good Lucy! I the work keeps improving!

class PointsHistoryResponseDtoTest {

@Test
void testDtoStructure() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I understand that testDtoStructure is for coverage/Sonar, but now that we have @JsonTest that validates the actual contract, could we remove this test and the class?

# Conflicts:
#	CHANGELOG.md
#	itachallenge-challenge/src/main/java/com/itachallenge/gamification/document/UserScoreDocument.java
#	itachallenge-challenge/src/main/java/com/itachallenge/gamification/repository/UserScoreRepository.java
#	itachallenge-challenge/src/test/java/com/itachallenge/gamification/document/UserScoreDocumentTest.java
#	itachallenge-challenge/src/test/java/com/itachallenge/gamification/repository/UserScoreRepositoryTest.java
# Conflicts:
#	CHANGELOG.md
@pantalois pantalois force-pushed the feature/190-2/point-history-retrival-logic branch from fce0295 to ced6d51 Compare February 25, 2026 11:53
@sonarqubecloud
Copy link

@ivilarop ivilarop merged commit 5ebec52 into develop Feb 25, 2026
3 checks passed
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