Skip to content

874: Migrated Question To Optional#862

Open
az2924 wants to merge 2 commits intomainfrom
874
Open

874: Migrated Question To Optional#862
az2924 wants to merge 2 commits intomainfrom
874

Conversation

@az2924
Copy link
Collaborator

@az2924 az2924 commented Mar 17, 2026

874

Description of changes

Checklist before review

  • I have done a thorough self-review of the PR
  • Copilot has reviewed my latest changes, and all comments have been fixed and/or closed.
  • If I have made database changes, I have made sure I followed all the db repo rules listed in the wiki here. (check if no db changes)
  • All tests have passed
  • I have successfully deployed this PR to staging
  • I have done manual QA in both dev (and staging if possible) and attached screenshots below.

Screenshots

Dev

  • None; simple type change

Staging

  • None; simple type change

@github-actions
Copy link
Contributor

Available PR Commands

  • /ai - Triggers all AI review commands at once
  • /review - AI review of the PR changes
  • /describe - AI-powered description of the PR
  • /improve - AI-powered suggestions
  • /deploy - Deploy to staging

See: https://github.com/tahminator/codebloom/wiki/CI-Commands

@github-actions
Copy link
Contributor

Title

874: Migrated Question To Optional


PR Type

Enhancement


Description

  • Migrated QuestionTopic model fields to Optional.

  • Updated repository methods to return Optional.

  • Refactored SQL queries for clarity.

  • Enhanced DTO serialization for Optional fields.


File Walkthrough

Relevant files
Enhancement
QuestionTopic.java
Migrate QuestionTopic fields to Optional                 

src/main/java/org/patinanetwork/codebloom/common/db/models/question/topic/QuestionTopic.java

  • Changed questionId and questionBankId fields to Optional.
  • Added @Builder.Default to initialize optional fields to
    Optional.empty().
  • Implemented custom builder methods to handle String inputs for
    optional fields.
+19/-2   
QuestionTopicRepository.java
Update QuestionTopicRepository methods to return Optional

src/main/java/org/patinanetwork/codebloom/common/db/repos/question/topic/QuestionTopicRepository.java

  • Updated findQuestionTopicById to return Optional.
  • Updated findQuestionTopicByQuestionIdAndTopicEnum to return Optional.
  • Adjusted Javadoc for createQuestionTopic and updateQuestionTopicById.
+4/-4     
QuestionTopicSqlRepository.java
Implement Optional handling in QuestionTopicSqlRepository

src/main/java/org/patinanetwork/codebloom/common/db/repos/question/topic/QuestionTopicSqlRepository.java

  • Implemented Optional return types for findQuestionTopicById and
    findQuestionTopicByQuestionIdAndTopicEnum.
  • Modified createQuestionTopic and updateQuestionTopicById to handle
    Optional for SQL parameter binding.
  • Reformatted SQL queries for improved readability.
  • Updated error messages and simplified deleteQuestionTopicById return.
+70/-127
QuestionTopicDto.java
Update QuestionTopicDto for Optional fields and JSON serialization

src/main/java/org/patinanetwork/codebloom/common/dto/question/topic/QuestionTopicDto.java

  • Changed questionId and questionBankId fields to Optional.
  • Added @JsonInclude(JsonInclude.Include.NON_EMPTY) for conditional JSON
    serialization.
  • Updated @Schema annotations to reflect NOT_REQUIRED for optional
    fields.
  • Included questionBankId in fromQuestionTopic mapping.
+9/-2     
Tests
QuestionTopicRepositoryTest.java
Update QuestionTopicRepository tests for Optional returns

src/test/java/org/patinanetwork/codebloom/common/db/repos/question/topic/QuestionTopicRepositoryTest.java

  • Adapted test methods to expect and handle Optional return types.
  • Updated assertions from assertNotNull to assertTrue(isPresent()).
  • Refined log and failure messages for clarity.
  • Made questionTopicRepository field final.
+23/-19 

@github-actions
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Database Schema

The PR migrates questionId and questionBankId to Optional<String> in the QuestionTopic model. While the @NullColumn annotation is present, it's important to verify that the actual database schema for the QuestionTopic table allows NULL values for these columns. If the columns are currently defined as NOT NULL, a database migration would be required to align the schema with the code changes.

private Optional<String> questionId = Optional.empty();

@NullColumn
@Builder.Default
private Optional<String> questionBankId = Optional.empty();

@az2924
Copy link
Collaborator Author

az2924 commented Mar 17, 2026

/deploy

@github-actions
Copy link
Contributor

Title

874: Migrated Question To Optional


PR Type

Enhancement


Description

  • Migrate questionId, questionBankId to Optional<String>.

  • Update QuestionTopicRepository methods for Optional returns.

  • Refactor QuestionTopicSqlRepository to handle Optional fields.

  • Adjust QuestionTopicDto for Optional and JSON serialization.


File Walkthrough

Relevant files
Enhancement
QuestionTopic.java
Update QuestionTopic model to use Optional fields               

src/main/java/org/patinanetwork/codebloom/common/db/models/question/topic/QuestionTopic.java

  • Changed questionId and questionBankId fields from String to Optional.
  • Initialized Optional fields with Optional.empty() using
    @Builder.Default.
  • Added custom QuestionTopicBuilder methods to wrap String inputs in
    Optional.ofNullable().
+19/-2   
QuestionTopicRepository.java
Update QuestionTopicRepository methods to return Optional

src/main/java/org/patinanetwork/codebloom/common/db/repos/question/topic/QuestionTopicRepository.java

  • Updated findQuestionTopicById to return Optional.
  • Updated findQuestionTopicByQuestionIdAndTopicEnum to return Optional.
+4/-2     
QuestionTopicSqlRepository.java
Implement Optional handling in QuestionTopicSqlRepository

src/main/java/org/patinanetwork/codebloom/common/db/repos/question/topic/QuestionTopicSqlRepository.java

  • Modified findQuestionTopicById and
    findQuestionTopicByQuestionIdAndTopicEnum to return Optional.of() the
    result or Optional.empty().
  • Refactored createQuestionTopic and updateQuestionTopicById to use
    Optional.map().orElse(null) for setting questionId and questionBankId
    in SQL statements.
  • Simplified SQL query string formatting for several methods.
+68/-125
QuestionTopicDto.java
Update QuestionTopicDto for Optional fields and JSON serialization

src/main/java/org/patinanetwork/codebloom/common/dto/question/topic/QuestionTopicDto.java

  • Added @JsonInclude(JsonInclude.Include.NON_EMPTY) to prevent
    serialization of empty Optional fields.
  • Changed questionId and questionBankId fields from String to Optional.
  • Updated @Schema annotations for questionId and questionBankId to
    NOT_REQUIRED.
  • Included questionBankId in the fromQuestionTopic builder method.
+9/-2     
Tests
QuestionTopicRepositoryTest.java
Adjust QuestionTopicRepository tests for Optional returns

src/test/java/org/patinanetwork/codebloom/common/db/repos/question/topic/QuestionTopicRepositoryTest.java

  • Updated testFindQuestionTopicById,
    testFindQuestionTopicByQuestionIdAndTopicEnum, and testUpdateQuestion
    to handle Optional return types.
  • Replaced assertNotNull checks with
    assertTrue(possibleQuestionTopic.isPresent()).
  • Accessed QuestionTopic objects using .get() on the Optional instances.
  • Corrected a typo in a fail message for
    testFindQuestionTopicsByQuestionBankId.
+20/-16 

@github-actions
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

API Contract Change

The questionId and questionBankId fields in QuestionTopicDto have been changed from Schema.RequiredMode.REQUIRED to Schema.RequiredMode.NOT_REQUIRED. While this is consistent with the migration to Optional, it represents a change in the API contract that consumers of this DTO should be aware of.

@Schema(requiredMode = Schema.RequiredMode.NOT_REQUIRED)
private Optional<String> questionId;

@Schema(requiredMode = Schema.RequiredMode.NOT_REQUIRED)
private Optional<String> questionBankId;

874: Update comments

874: Resolved warnings
@az2924
Copy link
Collaborator Author

az2924 commented Mar 17, 2026

/deploy

@github-actions
Copy link
Contributor

Title

874: Migrated Question To Optional


PR Type

Enhancement


Description

  • Migrate QuestionTopic fields to Optional<String>.

  • Update repository read methods to return Optional<QuestionTopic>.

  • Refactor SQL repository to handle Optional for nullability.

  • Adjust DTO and tests for Optional type safety.


File Walkthrough

Relevant files
Enhancement
QuestionTopic.java
Migrate QuestionTopic fields to Optional                                 

src/main/java/org/patinanetwork/codebloom/common/db/models/question/topic/QuestionTopic.java

  • Changed questionId and questionBankId fields from String to Optional.
  • Added custom builder methods to wrap String inputs for questionId and
    questionBankId into Optional.
+15/-2   
QuestionTopicRepository.java
Update QuestionTopicRepository methods to return Optional

src/main/java/org/patinanetwork/codebloom/common/db/repos/question/topic/QuestionTopicRepository.java

  • Updated findQuestionTopicById to return Optional.
  • Updated findQuestionTopicByQuestionIdAndTopicEnum to return Optional.
+4/-2     
QuestionTopicSqlRepository.java
Implement Optional handling in QuestionTopicSqlRepository

src/main/java/org/patinanetwork/codebloom/common/db/repos/question/topic/QuestionTopicSqlRepository.java

  • Introduced static final strings for column names (COL_ID,
    COL_QUESTION_ID, etc.).
  • Modified mapResultSetToQuestionTopic to use Optional.ofNullable for
    nullable fields.
  • Updated findQuestionTopicById and
    findQuestionTopicByQuestionIdAndTopicEnum to return Optional.of or
    Optional.empty.
  • Refactored createQuestionTopic and updateQuestionTopicById to handle
    Optional values when setting SQL parameters.
+86/-103
QuestionTopicDto.java
Update QuestionTopicDto for Optional fields and JSON serialization

src/main/java/org/patinanetwork/codebloom/common/dto/question/topic/QuestionTopicDto.java

  • Added @JsonInclude(JsonInclude.Include.NON_EMPTY) to the class for
    JSON serialization.
  • Changed questionId and questionBankId fields to Optional with
    Schema.RequiredMode.NOT_REQUIRED.
  • Updated fromQuestionTopic to correctly map Optional fields.
+9/-2     
Tests
QuestionTopicRepositoryTest.java
Adjust QuestionTopicRepository tests for Optional types   

src/test/java/org/patinanetwork/codebloom/common/db/repos/question/topic/QuestionTopicRepositoryTest.java

  • Updated test methods to handle Optional return types from repository
    methods.
  • Replaced assertNotNull with assertTrue(isPresent()) and accessed
    values using get().
  • Refined log and failure messages for clarity.
+20/-16 

@github-actions
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Builder Pattern

The custom builder methods for questionId and questionBankId correctly wrap the input strings in Optional.ofNullable(). This is a good practice when using Lombok's @Builder with Optional fields. Ensure this pattern is consistently applied across other models if they also utilize @Builder with Optional fields.

public static class QuestionTopicBuilder {
    public QuestionTopicBuilder questionId(String questionId) {
        this.questionId = Optional.ofNullable(questionId);
        return this;
    }

    public QuestionTopicBuilder questionBankId(String questionBankId) {
        this.questionBankId = Optional.ofNullable(questionBankId);
        return this;
    }
JSON Serialization

The @JsonInclude(JsonInclude.Include.NON_EMPTY) annotation is correctly applied to QuestionTopicDto. This ensures that Optional.empty() fields are omitted from the JSON output, which is generally desired for cleaner API responses. Verify that this behavior aligns with the intended API contract for these fields.

@JsonInclude(JsonInclude.Include.NON_EMPTY)

@github-actions
Copy link
Contributor

PR Code Suggestions ✨

No code suggestions found for the PR.

@az2924
Copy link
Collaborator Author

az2924 commented Mar 17, 2026

/deploy

@github-actions
Copy link
Contributor

Title

874: Migrated Question To Optional


PR Type

Enhancement


Description

  • Migrate QuestionTopic fields to Optional<String>.

  • Update QuestionTopicRepository methods to return Optional.

  • Adjust QuestionTopicDto for Optional fields and JSON inclusion.

  • Refactor QuestionTopicSqlRepository for Optional handling.


Diagram Walkthrough

flowchart LR
  A[QuestionTopic Model] -- "Fields to Optional<String>" --> B[QuestionTopic Builder]
  B -- "Handles Optional.ofNullable" --> C[QuestionTopic DTO]
  C -- "Optional fields, JsonInclude.NON_EMPTY" --> D[QuestionTopic Repository Interface]
  D -- "Methods return Optional<QuestionTopic>" --> E[QuestionTopic SQL Repository]
  E -- "Handles Optional for DB operations" --> F[QuestionTopic Repository Tests]
  F -- "Asserts Optional.isPresent()" --> G[Improved Null Safety]
Loading

File Walkthrough

Relevant files
Enhancement
QuestionTopic.java
Update QuestionTopic model to use Optional for nullable fields

src/main/java/org/patinanetwork/codebloom/common/db/models/question/topic/QuestionTopic.java

  • Changed questionId and questionBankId fields from String to Optional.
  • Added Optional import.
  • Implemented custom QuestionTopicBuilder methods to wrap questionId and
    questionBankId in Optional.ofNullable() and default to
    Optional.empty() if null.
+25/-2   
QuestionTopicRepository.java
Update QuestionTopicRepository methods to return Optional

src/main/java/org/patinanetwork/codebloom/common/db/repos/question/topic/QuestionTopicRepository.java

  • Modified findQuestionTopicById and
    findQuestionTopicByQuestionIdAndTopicEnum to return Optional.
  • Added Optional import.
+4/-2     
QuestionTopicSqlRepository.java
Refactor QuestionTopicSqlRepository for Optional handling

src/main/java/org/patinanetwork/codebloom/common/db/repos/question/topic/QuestionTopicSqlRepository.java

  • Introduced constants for database column names (COL_ID,
    COL_QUESTION_ID, etc.).
  • Updated findQuestionTopicById and
    findQuestionTopicByQuestionIdAndTopicEnum to return Optional.of() or
    Optional.empty().
  • Modified createQuestionTopic and updateQuestionTopicById to correctly
    handle Optional fields when setting SQL parameters.
  • Improved SQL query formatting and error messages.
+86/-103
QuestionTopicDto.java
Adjust QuestionTopicDto for Optional fields and JSON inclusion

src/main/java/org/patinanetwork/codebloom/common/dto/question/topic/QuestionTopicDto.java

  • Added @JsonInclude(JsonInclude.Include.NON_EMPTY) annotation to the
    class for JSON serialization.
  • Changed questionId and questionBankId fields to Optional and set their
    schema requiredMode to NOT_REQUIRED.
  • Updated fromQuestionTopic builder to include questionBankId.
+9/-2     
Tests
QuestionTopicRepositoryTest.java
Update QuestionTopicRepository tests for Optional return types

src/test/java/org/patinanetwork/codebloom/common/db/repos/question/topic/QuestionTopicRepositoryTest.java

  • Added Optional import and made questionTopicRepository field final.
  • Updated test methods (testFindQuestionTopicById,
    testFindQuestionTopicByQuestionIdAndTopicEnum, testUpdateQuestion) to
    assert Optional.isPresent() and retrieve values using get().
  • Corrected log and failure messages to reflect Optional usage and
    ensure accurate test outcomes.
+20/-16 

@github-actions
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Builder Logic

The custom QuestionTopicBuilder correctly handles Optional.ofNullable and ensures that questionId and questionBankId are initialized to Optional.empty() if not explicitly set. This is a good pattern for integrating Lombok's @Builder with Optional fields. Please verify that this logic is robust and covers all desired scenarios for constructing QuestionTopic objects.

public static class QuestionTopicBuilder {
    public QuestionTopicBuilder questionId(String questionId) {
        this.questionId = Optional.ofNullable(questionId);
        return this;
    }

    public QuestionTopicBuilder questionBankId(String questionBankId) {
        this.questionBankId = Optional.ofNullable(questionBankId);
        return this;
    }

    public QuestionTopic build() {
        if (this.questionId == null) {
            this.questionId = Optional.empty();
        }
        if (this.questionBankId == null) {
            this.questionBankId = Optional.empty();
        }
        return new QuestionTopic(id, questionId, questionBankId, topicSlug, topic, createdAt);
    }
}
Database Nullability

The migration to Optional<String> for questionId and questionBankId in the model and repository implies that the corresponding database columns should be nullable. While the PR description states no database changes, it's crucial to confirm that the underlying database schema for the QuestionTopic table allows NULL values for these columns to prevent runtime errors or data integrity issues.

stmt.setObject(
        COL_QUESTION_ID,
        questionTopic.getQuestionId().map(UUID::fromString).orElse(null));
stmt.setObject(
        COL_QUESTION_BANK_ID,
        questionTopic.getQuestionBankId().map(UUID::fromString).orElse(null));

@github-actions
Copy link
Contributor

PR Code Suggestions ✨

No code suggestions found for the PR.

public class QuestionTopicSqlRepository implements QuestionTopicRepository {

private final DataSource ds;
private static final String COL_ID = "id";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't this a little redundant?

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.

2 participants