Skip to content

[Refactor/use public id for record]#64

Merged
ekgns33 merged 5 commits intomainfrom
refactor/use-public-id-for-record
Apr 23, 2025
Merged

[Refactor/use public id for record]#64
ekgns33 merged 5 commits intomainfrom
refactor/use-public-id-for-record

Conversation

@ekgns33
Copy link
Copy Markdown
Contributor

@ekgns33 ekgns33 commented Apr 22, 2025

작업내역

API 변경

  • public_id 를 클라이언트에서 사용하도록 api-spec을 변경했습니다.
  • PATCH /api/v1/records/{id}에서 title, description 외의 정보들도 수정하던 로직을 수정했습니다.
    • 이제는 body로 title, description, img_url 만 수정합니다.

엔티티 수정

  • RunningRecord 엔티티에서 public_id를 생성자에서 직접할당하던 로직을 수정했습니다.

    • 이제는 @PrePersist를 활용하여 영속성 컨텍스트에 등록될때, 세팅됩니다.
  • 엔티티에 description, img_url 필드가 빠져있어 추가했습니다.

테스트코드를 수정했습니다.

  • recordId를 사용하던 api
    • /api/v1/records/**
    • /api/v1/rewards/runnings

Summary by CodeRabbit

  • New Features

    • Added support for updating running record descriptions and image URLs.
    • Enhanced API documentation with detailed schema annotations for record-related responses.
  • Improvements

    • Switched record identifiers from numeric IDs to UUID strings for better consistency and security.
    • Updated record details view to include total running time and improved field descriptions.
    • Added validation to ensure non-null and non-empty record identifiers and editor IDs during updates.
    • Introduced automatic UUID generation for new running records upon creation.
  • Bug Fixes

    • Adjusted tests and API requests to use the new UUID-based record identifiers.
  • Tests

    • Added tests for invalid pagination requests and updating user records.
    • Updated reward and record tests to align with new identifier format.

@ekgns33 ekgns33 added bug Something isn't working enhancement New feature or request labels Apr 22, 2025
@ekgns33 ekgns33 requested a review from jeeheaG April 22, 2025 10:46
@ekgns33 ekgns33 self-assigned this Apr 22, 2025
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 22, 2025

Walkthrough

This update refactors the handling of record identifiers across the running record and rewards modules, shifting from internal numeric IDs to public UUID-based string IDs throughout the API, DTOs, services, and tests. It updates method signatures, request/response objects, and validation annotations to consistently use public IDs. The update also modifies the record update process to accept and propagate the public ID, adjusts domain logic to support new update methods, and enhances API documentation with OpenAPI schema annotations. Corresponding changes are made to acceptance and service tests to reflect the new identifier type and update workflows.

Changes

File(s) Change Summary
src/main/java/org/runimo/runimo/records/controller/RecordController.java Updated updateRecord method to use @PathVariable String recordId (public ID) and pass it to the update command; added @NotNull annotation.
src/main/java/org/runimo/runimo/records/controller/request/RecordUpdateRequest.java Replaced timing and metric fields with description and imgUrl; updated toCommand to accept and use recordPublicId.
src/main/java/org/runimo/runimo/records/domain/RunningRecord.java Changed public ID assignment to use @PrePersist; added specific update methods for title, description, and image URL; removed generic update and static factory methods.
src/main/java/org/runimo/runimo/records/service/RecordCommandService.java Changed saveRecord to return RunningRecord entity; updated updateRecord to call new field-specific update methods; removed unused mapping method.
src/main/java/org/runimo/runimo/records/service/usecases/RecordCreateUsecaseImpl.java Modified execute to return RecordSaveResponse with public ID from saved record.
src/main/java/org/runimo/runimo/records/service/usecases/dtos/RecordDetailViewResponse.java Changed recordId field to String (public ID); added totalRunningTime and OpenAPI schema annotations; updated static factory method.
src/main/java/org/runimo/runimo/records/service/usecases/dtos/RecordSaveResponse.java Changed savedId to String (public ID); added OpenAPI schema annotations.
src/main/java/org/runimo/runimo/records/service/usecases/dtos/RecordUpdateCommand.java Added @NotEmpty to editorId and recordPublicId; replaced timing/metric fields with description and imgUrl.
src/main/java/org/runimo/runimo/rewards/controller/request/RewardClaimRequest.java Changed recordId field from Long to String (public ID); updated schema example.
src/main/java/org/runimo/runimo/rewards/service/RewardService.java Updated to find records and validate using public ID instead of internal ID.
src/main/java/org/runimo/runimo/rewards/service/dto/RewardClaimCommand.java Changed recordId field from Long to String (public ID).
src/test/java/org/runimo/runimo/records/api/RecordAcceptanceTest.java Updated assertions to check for non-null public IDs; added tests for pagination validation and record update using public ID.
src/test/java/org/runimo/runimo/rewards/api/RewardAcceptanceTest.java Updated tests to use string-based public IDs in requests and responses.
src/test/java/org/runimo/runimo/rewards/service/RewardServiceTest.java Updated test setup and mocks to use public IDs (UUID strings) instead of numeric IDs.
src/main/resources/sql/schema.sql and src/test/resources/sql/schema.sql Added description and img_url columns to running_record table schema.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant RecordController
    participant RecordCommandService
    participant RunningRecord

    Client->>RecordController: PATCH /records/{recordId} (RecordUpdateRequest)
    RecordController->>RecordCommandService: updateRecord(editorId, recordPublicId, request)
    RecordCommandService->>RunningRecord: updateTitle(editorId, title)
    RecordCommandService->>RunningRecord: updateDescription(editorId, description)
    RecordCommandService->>RunningRecord: updateImageUrl(editorId, imgUrl)
    RecordCommandService-->>RecordController: void
    RecordController-->>Client: 200 OK
Loading

Suggested reviewers

  • jeeheaG

Poem

In burrows deep, a record hides,
Now found by UUIDs, not numbers' tides.
With titles, pics, and words anew,
The update flows both swift and true!
Rewards and runs, all now aligned—
A string of changes, neatly twined.
🐇✨ Cheers to code that hops ahead!


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e7da1d5 and c8844fe.

📒 Files selected for processing (16)
  • src/main/java/org/runimo/runimo/records/controller/RecordController.java (2 hunks)
  • src/main/java/org/runimo/runimo/records/controller/request/RecordUpdateRequest.java (1 hunks)
  • src/main/java/org/runimo/runimo/records/domain/RunningRecord.java (5 hunks)
  • src/main/java/org/runimo/runimo/records/service/RecordCommandService.java (1 hunks)
  • src/main/java/org/runimo/runimo/records/service/usecases/RecordCreateUsecaseImpl.java (1 hunks)
  • src/main/java/org/runimo/runimo/records/service/usecases/dtos/RecordDetailViewResponse.java (1 hunks)
  • src/main/java/org/runimo/runimo/records/service/usecases/dtos/RecordSaveResponse.java (1 hunks)
  • src/main/java/org/runimo/runimo/records/service/usecases/dtos/RecordUpdateCommand.java (1 hunks)
  • src/main/java/org/runimo/runimo/rewards/controller/request/RewardClaimRequest.java (1 hunks)
  • src/main/java/org/runimo/runimo/rewards/service/RewardService.java (2 hunks)
  • src/main/java/org/runimo/runimo/rewards/service/dto/RewardClaimCommand.java (1 hunks)
  • src/main/resources/sql/schema.sql (1 hunks)
  • src/test/java/org/runimo/runimo/records/api/RecordAcceptanceTest.java (4 hunks)
  • src/test/java/org/runimo/runimo/rewards/api/RewardAcceptanceTest.java (6 hunks)
  • src/test/java/org/runimo/runimo/rewards/service/RewardServiceTest.java (5 hunks)
  • src/test/resources/sql/schema.sql (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • src/test/resources/sql/schema.sql
🚧 Files skipped from review as they are similar to previous changes (15)
  • src/main/java/org/runimo/runimo/rewards/service/dto/RewardClaimCommand.java
  • src/main/resources/sql/schema.sql
  • src/test/java/org/runimo/runimo/rewards/api/RewardAcceptanceTest.java
  • src/main/java/org/runimo/runimo/records/service/usecases/RecordCreateUsecaseImpl.java
  • src/main/java/org/runimo/runimo/rewards/controller/request/RewardClaimRequest.java
  • src/main/java/org/runimo/runimo/records/service/usecases/dtos/RecordSaveResponse.java
  • src/main/java/org/runimo/runimo/records/controller/RecordController.java
  • src/test/java/org/runimo/runimo/rewards/service/RewardServiceTest.java
  • src/main/java/org/runimo/runimo/rewards/service/RewardService.java
  • src/test/java/org/runimo/runimo/records/api/RecordAcceptanceTest.java
  • src/main/java/org/runimo/runimo/records/service/RecordCommandService.java
  • src/main/java/org/runimo/runimo/records/domain/RunningRecord.java
  • src/main/java/org/runimo/runimo/records/controller/request/RecordUpdateRequest.java
  • src/main/java/org/runimo/runimo/records/service/usecases/dtos/RecordDetailViewResponse.java
  • src/main/java/org/runimo/runimo/records/service/usecases/dtos/RecordUpdateCommand.java

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🔭 Outside diff range comments (1)
src/main/java/org/runimo/runimo/records/service/usecases/dtos/RecordDetailViewResponse.java (1)

31-41: 🛠️ Refactor suggestion

⚠️ Potential issue

imgUrl is never populated – DTO returns null to the client

The from() factory sets every field except imgUrl, so the API will always respond with imgUrl = null even when the underlying entity holds a value. This is an easy thing to miss because the build still succeeds – Lombok’s @Builder just leaves the field unset.

             .segmentPaceList(runningRecord.getPacePerKm())
+            .imgUrl(runningRecord.getImgUrl())   // ← missing mapping
             .build();

Please map the value (or drop the field entirely if it is no longer required) to keep the contract with clients consistent.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b2fcfee and 1d14396.

📒 Files selected for processing (14)
  • src/main/java/org/runimo/runimo/records/controller/RecordController.java (2 hunks)
  • src/main/java/org/runimo/runimo/records/controller/request/RecordUpdateRequest.java (1 hunks)
  • src/main/java/org/runimo/runimo/records/domain/RunningRecord.java (3 hunks)
  • src/main/java/org/runimo/runimo/records/service/RecordCommandService.java (1 hunks)
  • src/main/java/org/runimo/runimo/records/service/usecases/RecordCreateUsecaseImpl.java (1 hunks)
  • src/main/java/org/runimo/runimo/records/service/usecases/dtos/RecordDetailViewResponse.java (1 hunks)
  • src/main/java/org/runimo/runimo/records/service/usecases/dtos/RecordSaveResponse.java (1 hunks)
  • src/main/java/org/runimo/runimo/records/service/usecases/dtos/RecordUpdateCommand.java (1 hunks)
  • src/main/java/org/runimo/runimo/rewards/controller/request/RewardClaimRequest.java (1 hunks)
  • src/main/java/org/runimo/runimo/rewards/service/RewardService.java (2 hunks)
  • src/main/java/org/runimo/runimo/rewards/service/dto/RewardClaimCommand.java (1 hunks)
  • src/test/java/org/runimo/runimo/records/api/RecordAcceptanceTest.java (4 hunks)
  • src/test/java/org/runimo/runimo/rewards/api/RewardAcceptanceTest.java (6 hunks)
  • src/test/java/org/runimo/runimo/rewards/service/RewardServiceTest.java (5 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/main/java/org/runimo/runimo/rewards/controller/request/RewardClaimRequest.java (1)
src/main/java/org/runimo/runimo/records/service/dto/RecordSimpleView.java (1)
  • Schema (11-45)
src/main/java/org/runimo/runimo/records/service/usecases/dtos/RecordSaveResponse.java (1)
src/main/java/org/runimo/runimo/records/service/dto/RecordSimpleView.java (1)
  • Schema (11-45)
src/main/java/org/runimo/runimo/records/service/usecases/dtos/RecordDetailViewResponse.java (1)
src/main/java/org/runimo/runimo/records/service/dto/RecordSimpleView.java (1)
  • Schema (11-45)
src/main/java/org/runimo/runimo/records/controller/request/RecordUpdateRequest.java (1)
src/main/java/org/runimo/runimo/records/service/dto/RecordSimpleView.java (1)
  • Schema (11-45)
🔇 Additional comments (30)
src/main/java/org/runimo/runimo/rewards/service/dto/RewardClaimCommand.java (1)

5-5: Field type and name updated to use public ID instead of record ID

The field has been correctly updated from Long recordId to String recordPublicId, aligning with the PR objective of using public IDs in entity logic. This change ensures consistency with the broader refactoring across the codebase.

src/main/java/org/runimo/runimo/rewards/controller/request/RewardClaimRequest.java (1)

7-7: Updated field type and schema documentation

The type change from Long to String for recordId is appropriate and matches the PR objective. The schema example has been correctly updated to reflect the new UUID-based identification scheme. This ensures proper API documentation and client usage.

src/test/java/org/runimo/runimo/rewards/api/RewardAcceptanceTest.java (6)

73-76: Updated ID extraction and request creation to use String

The code now correctly extracts the record ID as a String instead of an Integer, and directly passes it to the RewardClaimRequest constructor without type conversion. This aligns with the new public ID implementation.


105-108: Consistent ID handling in multiple test cases

The same pattern of String extraction and direct usage is applied here, maintaining consistency across test cases.


155-155: String ID extraction in edge case test

The extraction of the record ID as a String is correctly implemented in this edge case test.


177-177: Updated RewardClaimRequest creation with String ID

The RewardClaimRequest constructor is now correctly called with a String parameter.


210-213: Consistent String ID handling in reward point test

This test case also correctly implements the extraction and usage of the String ID.


250-253: Refactored ID handling in edge case test for minimal distance

The extraction of record ID as a String and its usage in the RewardClaimRequest is consistently implemented in this test case as well.

src/main/java/org/runimo/runimo/records/service/usecases/dtos/RecordSaveResponse.java (1)

3-9: Added OpenAPI schema annotations and updated type to String

These changes accomplish two important goals:

  1. The type change from Long to String for savedId aligns with the public ID refactoring
  2. The addition of OpenAPI schema annotations improves API documentation

The updated example value "UUID-String" clearly communicates the expected format to API consumers.

src/test/java/org/runimo/runimo/rewards/service/RewardServiceTest.java (8)

16-16: Added necessary UUID import for new implementation.

This import addition is correct to support using UUID string identifiers for records.


54-55: Properly setting recordPublicId in test fixture.

The test fixture now correctly sets a random UUID string for the recordPublicId field, which aligns with the new approach of using public IDs for record identification.


71-71: Correctly updated RewardClaimCommand to use recordPublicId.

The command object initialization has been properly updated to use the public ID string from the running record instead of the numeric ID.


73-73: Updated mock to use findByPublicId instead of findById.

This correctly reflects the change in the service implementation to look up records by their public ID rather than internal ID.


88-89: Command creation properly uses record's public ID.

The command object is correctly initialized with the public ID from the record rather than its internal ID.


91-91: Mock setup correctly uses findByPublicId.

The mock is properly configured to respond to the new repository method for finding records by public ID.


105-105: Command now uses public ID in test for non-first-record scenario.

The test correctly uses the record's public ID when constructing the command for the non-first-record scenario.


107-107: Mock setup consistent with service implementation changes.

The mock configuration is properly updated to reflect the service's use of findByPublicId instead of findById.

src/main/java/org/runimo/runimo/rewards/service/RewardService.java (2)

32-32: Service now retrieves records by public ID.

The service implementation has been correctly updated to retrieve records using their public ID instead of internal ID, which aligns with the API's changes to use public identifiers.


64-64: Updated record comparison to use public IDs.

The validation method now correctly compares the record public IDs instead of internal IDs, ensuring consistent handling of record identifiers throughout the service layer.

src/main/java/org/runimo/runimo/records/service/usecases/RecordCreateUsecaseImpl.java (1)

36-36: Return response initialized with record's public ID.

The implementation now correctly returns a RecordSaveResponse initialized with the public ID from the saved record. This change aligns with the broader refactoring to use public IDs as primary external identifiers.

src/test/java/org/runimo/runimo/records/api/RecordAcceptanceTest.java (4)

8-8: Added import for JSON exception handling.

This import is needed to support the new test for record updates that uses JSON serialization.


20-20: Added import for RecordUpdateRequest.

This import is necessary for the new test that verifies record update functionality.


134-134: Updated assertion to check for non-null record ID.

The test now correctly verifies that the returned record ID is not null, rather than assuming it's a numeric value. This aligns with the change to use string-based public IDs.


287-322: Added comprehensive tests for pagination error handling and record updates.

These tests are valuable additions that validate:

  1. Error handling for invalid pagination parameters
  2. The record update functionality using public IDs in the request path

These tests help ensure the robustness of the API changes and provide coverage for the new update endpoint functionality.

src/main/java/org/runimo/runimo/records/service/usecases/dtos/RecordUpdateCommand.java (1)

3-14: LGTM - Appropriate validations for required fields

The changes correctly implement validation constraints on required fields while keeping description and imgUrl optional. This aligns well with the PR objective of transitioning to public IDs.

src/main/java/org/runimo/runimo/records/service/RecordCommandService.java (2)

19-21: Return type change improves code flow

Returning the RunningRecord entity directly from the repository save operation simplifies the code and provides more flexibility for callers.


29-33: Explicit field updates improve clarity and maintainability

The explicit calls to field-specific update methods (updateTitle, updateDescription, updateImageUrl) make the code more readable and clearly define the permitted update operations. This aligns well with the PR objective of restricting updates to only title, description, and img_url fields.

src/main/java/org/runimo/runimo/records/controller/RecordController.java (2)

9-9: Proper validation for path variable

The @NotNull annotation appropriately ensures that the recordId path variable is provided in requests.

Also applies to: 94-94


97-97: Successfully integrated public ID into update flow

The controller now correctly passes the string-based public ID to the command builder, aligning with the PR objective of using public_id in the API.

src/main/java/org/runimo/runimo/records/controller/request/RecordUpdateRequest.java (1)

11-14: Schema annotations added correctly for new fields

The new fields description and imgUrl are properly documented with appropriate Swagger annotations.

Comment thread src/main/java/org/runimo/runimo/records/domain/RunningRecord.java
@ekgns33 ekgns33 mentioned this pull request Apr 22, 2025
@ekgns33 ekgns33 force-pushed the refactor/use-public-id-for-record branch from e7da1d5 to c8844fe Compare April 22, 2025 11:07
@ekgns33 ekgns33 merged commit 056d745 into main Apr 23, 2025
4 checks passed
@ekgns33 ekgns33 deleted the refactor/use-public-id-for-record branch April 23, 2025 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant