Skip to content

240: Create new filters param on previous leaderboard endpoint#909

Open
MalihaT111 wants to merge 1 commit into
mainfrom
240
Open

240: Create new filters param on previous leaderboard endpoint#909
MalihaT111 wants to merge 1 commit into
mainfrom
240

Conversation

@MalihaT111
Copy link
Copy Markdown
Contributor

@MalihaT111 MalihaT111 commented Apr 18, 2026

240

Description of changes

Added filters parameter (a set of strings) that store the items that belong in the leaderboard url. Added an if else statement for when filters is empty or null. When it's empty or null it goes back to previous url.

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

Current:
image

New:
image

Staging

@github-actions
Copy link
Copy Markdown
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
Copy Markdown
Contributor

Ticket Validation Failed

The following issues were found with the attached ticket:

  • Acceptance Criteria (AC) is not provided in the ticket description.
  • Ticket does not have a priority defined.

@github-actions
Copy link
Copy Markdown
Contributor

Title

240: Create new filters param on previous leaderboard endpoint


PR Type

Enhancement


Description

  • Introduce filters parameter for leaderboard queries.

  • Allow filtering by comma-separated list of tags.

  • Conditionally apply filters based on filters presence.

  • Deprecate individual boolean filter parameters.


Diagram Walkthrough

flowchart LR
  A[Leaderboard API Call] --> B{Filters parameter provided?};
  B -- Yes --> C[Parse filters set];
  B -- No --> D[Use individual boolean parameters];
  C --> E[Build LeaderboardFilterOptions];
  D --> E;
  E --> F[Get leaderboard users];
Loading

File Walkthrough

Relevant files
Enhancement
LeaderboardController.java
Add unified `filters` parameter to leaderboard endpoint   

src/main/java/org/patinanetwork/codebloom/api/leaderboard/LeaderboardController.java

  • Added an optional filters request parameter of type Set to the
    getLeaderboardUsersById method.
  • Implemented conditional logic to construct LeaderboardFilterOptions
    based on the presence of the filters parameter.
  • If filters is provided, individual filter flags (e.g., patina, hunter)
    are determined by checking if the filters set contains the
    corresponding string.
  • If filters is null or empty, the method falls back to using the
    existing individual boolean request parameters for filtering.
+46/-20 

@github-actions
Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅

240 - PR Code Verified

Compliant requirements:

  • The backend endpoint now supports a new filters parameter for dynamic filtering.
  • The backend correctly handles cases where no filters are provided, maintaining backward compatibility.

Requires further human verification:

  • Verification of the leaderboard list display on the admin page.
  • Verification of the filtering functionality on the admin page through UI interaction.
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Missing Test Coverage

The new if/else logic introduced to parse the filters parameter and construct the LeaderboardFilterOptions is not covered by dedicated tests. This logic determines how various filter combinations are applied and how the system falls back to previous behavior when no filters are provided. Without specific tests, there's a risk of regressions or incorrect behavior if the parsing logic is modified in the future.

if (filters == null || filters.isEmpty()) {
    globalIndexNew = globalIndex;
    options = LeaderboardFilterOptions.builder()
            .page(page)
            .pageSize(parsedPageSize)
            .query(query)
            .patina(patina)
            .hunter(hunter)
            .nyu(nyu)
            .baruch(baruch)
            .rpi(rpi)
            .gwc(gwc)
            .sbu(sbu)
            .ccny(ccny)
            .columbia(columbia)
            .cornell(cornell)
            .bmcc(bmcc)
            .mhcplusplus(mhcplusplus)
            .build();
} else {
    globalIndexNew = filters.contains("globalIndex");
    options = LeaderboardFilterOptions.builder()
            .page(page)
            .pageSize(parsedPageSize)
            .query(query)
            .patina(filters.contains("patina"))
            .hunter(filters.contains("hunter"))
            .nyu(filters.contains("nyu"))
            .baruch(filters.contains("baruch"))
            .rpi(filters.contains("rpi"))
            .gwc(filters.contains("gwc"))
            .sbu(filters.contains("sbu"))
            .ccny(filters.contains("ccny"))
            .columbia(filters.contains("columbia"))
            .cornell(filters.contains("cornell"))
            .bmcc(filters.contains("bmcc"))
            .mhcplusplus(filters.contains("mhcplusplus"))
            .build();

@github-actions
Copy link
Copy Markdown
Contributor

Ticket Validation Failed

The following issues were found with the attached ticket:

  • Acceptance Criteria (AC) is not provided in the ticket description.

@github-actions
Copy link
Copy Markdown
Contributor

Title

240: Create new filters param on previous leaderboard endpoint


PR Type

Enhancement


Description

  • Introduce filters query parameter.

  • Unify leaderboard filter processing.

  • Ensure backward compatibility.

  • Update related unit tests.


Diagram Walkthrough

flowchart LR
  Client --> LeaderboardController["LeaderboardController.java"]
  LeaderboardController -- "Processes 'filters' param" --> LeaderboardFilterOptions["LeaderboardFilterOptions (built)"]
  LeaderboardFilterOptions --> LeaderboardManager["LeaderboardManager.getLeaderboardUsers"]
Loading

File Walkthrough

Relevant files
Enhancement
LeaderboardController.java
Refactor leaderboard user filtering to use unified filters parameter

src/main/java/org/patinanetwork/codebloom/api/leaderboard/LeaderboardController.java

  • Added a new @RequestParam filters of type Set to the
    getLeaderboardUsersById method, allowing a comma-separated list of
    filters.
  • Implemented conditional logic to construct LeaderboardFilterOptions:
    if filters is provided, it parses values from the set; otherwise, it
    uses existing boolean parameters.
  • Updated the globalIndex variable and LeaderboardFilterOptions builder
    to reflect the new filter parsing logic.
+46/-20 
Tests
LeaderboardControllerTest.java
Update leaderboard controller tests for new filter parameter

src/test/java/org/patinanetwork/codebloom/api/leaderboard/LeaderboardControllerTest.java

  • Modified existing calls to
    leaderboardController.getLeaderboardUsersById in unit tests.
  • Added null as the argument for the newly introduced filters parameter
    to maintain test compatibility.
+3/-0     

@github-actions
Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

240 - Partially compliant

Compliant requirements:

  • Allow filtering of the leaderboard list via a new filters parameter on the backend API.
  • Maintain backward compatibility for existing individual boolean filter parameters when the filters parameter is not provided.

Non-compliant requirements:

  • The PR does not directly implement the display of a leaderboard list on an admin page; this is a frontend task that would consume this backend API change.

Requires further human verification:

  • Verify that the new filters parameter correctly applies the intended filtering logic (e.g., globalIndex, patina, hunter, etc.) when provided.
  • Verify that the existing individual boolean parameters still work as expected when the filters parameter is null or empty.
  • Verify the integration of this new filtering functionality into the admin page (frontend).
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Missing Test Coverage

The new logic introduced by the filters parameter (specifically the else block where filters is not null or empty) is not covered by new tests. Existing tests were only updated to pass null for the filters parameter, which only covers the backward-compatible path. This leaves the core new functionality untested.

} else {
    globalIndexNew = filters.contains("globalIndex");
    options = LeaderboardFilterOptions.builder()
            .page(page)
            .pageSize(parsedPageSize)
            .query(query)
            .patina(filters.contains("patina"))
            .hunter(filters.contains("hunter"))
            .nyu(filters.contains("nyu"))
            .baruch(filters.contains("baruch"))
            .rpi(filters.contains("rpi"))
            .gwc(filters.contains("gwc"))
            .sbu(filters.contains("sbu"))
            .ccny(filters.contains("ccny"))
            .columbia(filters.contains("columbia"))
            .cornell(filters.contains("cornell"))
            .bmcc(filters.contains("bmcc"))
            .mhcplusplus(filters.contains("mhcplusplus"))
            .build();

@github-actions
Copy link
Copy Markdown
Contributor

Ticket Validation Failed

The following issues were found with the attached ticket:

  • Acceptance Criteria (AC) is not provided in the ticket description.

@github-actions
Copy link
Copy Markdown
Contributor

Title

240: Create new filters param on previous leaderboard endpoint


PR Type

Enhancement


Description

  • Add optional filters parameter to leaderboard endpoint.

  • Enable dynamic filtering using a set of string values.

  • Maintain backward compatibility with existing boolean parameters.

  • Update tests to accommodate the new optional parameter.


Diagram Walkthrough

flowchart LR
  A[Client] --> B[Leaderboard Endpoint];
  B -- New 'filters' param --> C{Filter Logic};
  C --> D[LeaderboardManager];
  D --> E[Filtered Data];
Loading

File Walkthrough

Relevant files
Enhancement
LeaderboardController.java
Add dynamic `filters` parameter to leaderboard endpoint   

src/main/java/org/patinanetwork/codebloom/api/leaderboard/LeaderboardController.java

  • Introduced an optional filters request parameter of type Set to
    getLeaderboardUsersById.
  • Implemented conditional logic to build LeaderboardFilterOptions based
    on the presence of filters.
  • If filters is provided, individual filter booleans are set by checking
    filters content.
  • If filters is null or empty, the existing individual boolean
    parameters are used.
+46/-20 
Tests
LeaderboardControllerTest.java
Update tests for new optional `filters` parameter               

src/test/java/org/patinanetwork/codebloom/api/leaderboard/LeaderboardControllerTest.java

  • Modified calls to leaderboardController.getLeaderboardUsersById in
    test methods.
  • Added null as the argument for the newly introduced filters parameter.
  • Ensured existing test cases continue to validate previous leaderboard
    filtering behavior.
+3/-0     

@github-actions
Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

240 - Partially compliant

Compliant requirements:

  • Add a filters parameter (a set of strings) to the leaderboard endpoint.
  • Implement logic to handle filters being empty or null, reverting to previous behavior.
  • Allow filtering by specific criteria (e.g., nyu, hunter, globalIndex).

Non-compliant requirements:

  • Add a leaderboard list to the admin page. (The PR implements filtering for an existing leaderboard endpoint, not adding a new list to an admin page as suggested by the GitHub issue title and screenshot.)

Requires further human verification:

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

Incomplete Test Coverage

The PR introduces a new filters parameter and associated logic to build LeaderboardFilterOptions. However, there are no dedicated test cases to verify that when the filters parameter is provided (e.g., containing 'nyu' or 'globalIndex'), the LeaderboardFilterOptions are correctly constructed and passed to the leaderboardManager.getLeaderboardUsers method. Existing tests only pass null for the filters parameter, which covers the fallback scenario but not the primary new functionality.

void setup() {
    when(leaderboardManager.getLeaderboardUsers(eq(LEADERBOARD_ID), any(), anyBoolean()))
            .thenReturn(null);
}

@Test
void testGetLeaderboardUsersByIdMhcPlusPlus() {
    leaderboardController.getLeaderboardUsersById(
            LEADERBOARD_ID,
            null,
            PAGE,
            PAGE_SIZE,
            "",
            false,
            false,
            false,
            false,
            false,
            false,
            false,
            false,
            false,
            false,
            false,
            true,
            false,
            null);

    ArgumentCaptor<LeaderboardFilterOptions> optsCaptor = ArgumentCaptor.forClass(LeaderboardFilterOptions.class);
    verify(leaderboardManager, times(1)).getLeaderboardUsers(eq(LEADERBOARD_ID), optsCaptor.capture(), eq(false));
    var opts = optsCaptor.getValue();

    assertNotNull(opts);
    assertTrue(opts.isMhcplusplus());
    assertEquals(PAGE_SIZE, opts.getPageSize());
    assertEquals(PAGE, opts.getPage());
}

@Test
void testCurrentLeaderboardUsersMhcPlusPlus() {
    when(leaderboardRepository.getRecentLeaderboardMetadata())
            .thenReturn(Optional.of(Leaderboard.builder().id(LEADERBOARD_ID).build()));

    leaderboardController.getCurrentLeaderboardUsers(
            null, PAGE, PAGE_SIZE, "", false, false, false, false, false, false, false, false, false, false, false,
            true, false);

    ArgumentCaptor<LeaderboardFilterOptions> optsCaptor = ArgumentCaptor.forClass(LeaderboardFilterOptions.class);
    verify(leaderboardManager, times(1)).getLeaderboardUsers(eq(LEADERBOARD_ID), optsCaptor.capture(), eq(false));
    var opts = optsCaptor.getValue();

    assertNotNull(opts);
    assertTrue(opts.isMhcplusplus());
    assertEquals(PAGE_SIZE, opts.getPageSize());
    assertEquals(PAGE, opts.getPage());
}

@Test
@DisplayName("returns 200 with metadata when leaderboard exists")
void returnsMetadata() {
    Leaderboard lb = Leaderboard.builder()
            .id(LEADERBOARD_ID)
            .name("Season 1")
            .createdAt(LocalDateTime.of(2025, 1, 1, 0, 0))
            .build();
    when(leaderboardManager.getLeaderboardMetadata(LEADERBOARD_ID)).thenReturn(Optional.of(lb));

    ResponseEntity<ApiResponder<LeaderboardDto>> response =
            leaderboardController.getLeaderboardMetadataByLeaderboardId(LEADERBOARD_ID, null);

    assertEquals(HttpStatus.OK, response.getStatusCode());
    assertTrue(response.getBody().isSuccess());
    assertEquals("Season 1", response.getBody().getPayload().getName());
}

@Test
@DisplayName("throws 404 when leaderboard does not exist")
void throws404WhenNotFound() {
    when(leaderboardManager.getLeaderboardMetadata("missing-id")).thenReturn(Optional.empty());

    ResponseStatusException ex = assertThrows(
            ResponseStatusException.class,
            () -> leaderboardController.getLeaderboardMetadataByLeaderboardId("missing-id", null));
    assertEquals(HttpStatus.NOT_FOUND, ex.getStatusCode());
}

@Test
@DisplayName("caps page size to MAX_LEADERBOARD_PAGE_SIZE (20)")
void capsPageSize() {
    leaderboardController.getLeaderboardUsersById(
            LEADERBOARD_ID,
            null,
            1,
            999,
            "",
            false,
            false,
            false,
            false,
            false,
            false,
            false,
            false,
            false,
            false,
            false,
            false,
            false,
            null);

    ArgumentCaptor<LeaderboardFilterOptions> captor = ArgumentCaptor.forClass(LeaderboardFilterOptions.class);
    verify(leaderboardManager).getLeaderboardUsers(eq(LEADERBOARD_ID), captor.capture(), eq(false));

    assertEquals(PAGE_SIZE, captor.getValue().getPageSize());
}

@Test
@DisplayName("passes all filter flags through to LeaderboardFilterOptions")
void passesAllFilters() {
    leaderboardController.getLeaderboardUsersById(
            LEADERBOARD_ID,
            null,
            2,
            10,
            "tahmid",
            true /* patina */,

@github-actions
Copy link
Copy Markdown
Contributor

Title

240: Create new filters param on previous leaderboard endpoint


PR Type

Enhancement, Tests


Description

  • Add filters parameter to leaderboard endpoint.

  • Prioritize filters for constructing query options.

  • Fall back to individual boolean filters.

  • Update and add unit tests.


Diagram Walkthrough

flowchart LR
  A[LeaderboardController.getLeaderboardUsersById] --> B{Is 'filters' parameter null or empty?};
  B -- Yes --> C[Use individual boolean parameters];
  B -- No --> D[Parse filters Set for options];
  C --> E[Build LeaderboardFilterOptions];
  D --> E;
  E --> F[Call leaderboardManager.getLeaderboardUsers];
Loading

File Walkthrough

Relevant files
Enhancement
LeaderboardController.java
Add `filters` parameter to leaderboard endpoint                   

src/main/java/org/patinanetwork/codebloom/api/leaderboard/LeaderboardController.java

  • Added a new @RequestParam final Set filters to the
    getLeaderboardUsersById method.
  • Implemented conditional logic to use the filters Set if provided,
    otherwise fall back to individual boolean parameters.
  • Updated globalIndex to be derived from the filters Set or its
    dedicated boolean parameter.
+46/-20 
Tests
LeaderboardControllerTest.java
Update and add tests for new leaderboard filters parameter

src/test/java/org/patinanetwork/codebloom/api/leaderboard/LeaderboardControllerTest.java

  • Modified existing test calls to getLeaderboardUsersById to include
    null for the new filters parameter.
  • Added new test cases to verify the functionality of the filters Set,
    including its priority over individual boolean parameters and fallback
    behavior.
  • Ensured globalIndex is correctly handled when filters are provided.
+109/-6 

@github-actions
Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

240 - Partially compliant

Compliant requirements:

  • Create a new filters parameter on the existing leaderboard endpoint.
  • The endpoint handles the filters parameter being empty or null by falling back to previous filtering logic.
  • The endpoint allows filtering of leaderboard users based on the provided filters set.

Non-compliant requirements:

  • None

Requires further human verification:

  • UI/UX verification: Ensure the new filters parameter is correctly integrated and displayed on the admin page as shown in the ticket and PR screenshots.
  • Functional testing: Verify that applying different filters via the UI correctly updates the leaderboard results.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ No major issues detected

@github-actions
Copy link
Copy Markdown
Contributor

Title

240: Create new filters param on previous leaderboard endpoint


PR Type

Enhancement


Description

  • Add optional filters parameter to leaderboard API

  • Process filters or fallback to individual booleans

  • Update leaderboard manager call with new global index

  • Enhance tests for new filter logic and fallbacks


Diagram Walkthrough

flowchart LR
  A[LeaderboardController.getLeaderboardUsersById] --> B{Is 'filters' parameter provided and not empty?};
  B -- Yes --> C[Build LeaderboardFilterOptions from 'filters' Set];
  B -- No/Empty --> D[Build LeaderboardFilterOptions from individual boolean parameters];
  C --> E[Call leaderboardManager.getLeaderboardUsers];
  D --> E;
Loading

File Walkthrough

Relevant files
Api changes
LeaderboardController.java
Add optional filter set to leaderboard endpoint                   

src/main/java/org/patinanetwork/codebloom/api/leaderboard/LeaderboardController.java

  • Introduced an optional filters request parameter of type Set to the
    getLeaderboardUsersById endpoint.
  • Added conditional logic to construct LeaderboardFilterOptions: either
    from the new filters set or by falling back to existing individual
    boolean parameters if filters is null or empty.
  • Updated the globalIndex parameter passed to
    leaderboardManager.getLeaderboardUsers based on the new filter logic.
+46/-20 
Tests
LeaderboardControllerTest.java
Update and add tests for new leaderboard filter logic       

src/test/java/org/patinanetwork/codebloom/api/leaderboard/LeaderboardControllerTest.java

  • Modified existing test methods to accommodate the new filters
    parameter by passing null.
  • Added usesFiltersSetWhenProvided test to verify correct parsing of the
    filters set.
  • Included setsGlobalIndexFromFiltersSet test to confirm globalIndex is
    correctly derived from the filters set.
  • Implemented fallsBackToBooleansWhenFiltersEmpty test to ensure
    fallback to individual boolean parameters when filters is empty.
+94/-0   

@github-actions
Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

**🎫 Ticket compliance analysis **

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

API Design Redundancy

The getLeaderboardUsersById endpoint now accepts two overlapping ways to specify filters: a Set<String> parameter named filters and multiple individual boolean query parameters (e.g., patina, hunter). While the code correctly implements precedence (prioritizing filters when present), this dual approach can make the API contract more complex and potentially confusing for consumers. Consider deprecating or removing the individual boolean flags in favor of the new filters parameter for a cleaner API.

public ResponseEntity<ApiResponder<Page<Indexed<UserWithScoreDto>>>> getLeaderboardUsersById(
        @PathVariable final String leaderboardId,
        @Parameter(description = "Comma-separated list of active filters.", example = "nyu,hunter,globalIndex")
                @RequestParam(required = false)
                final Set<String> filters,
        @Parameter(description = "Page index", example = "1") @RequestParam(required = false, defaultValue = "1")
                final int page,
        @Parameter(description = "Page size (maximum of " + MAX_LEADERBOARD_PAGE_SIZE)
                @RequestParam(required = false, defaultValue = "" + MAX_LEADERBOARD_PAGE_SIZE)
                final int pageSize,
        @Parameter(description = "Discord name", example = "tahmid")
                @RequestParam(required = false, defaultValue = "")
                final String query,
        @Parameter(description = "Filter for Patina users") @RequestParam(required = false, defaultValue = "false")
                final boolean patina,
        @Parameter(description = "Filter for Hunter College users")
                @RequestParam(required = false, defaultValue = "false")
                final boolean hunter,
        @Parameter(description = "Filter for NYU users") @RequestParam(required = false, defaultValue = "false")
                final boolean nyu,
        @Parameter(description = "Filter for Baruch College users")
                @RequestParam(required = false, defaultValue = "false")
                final boolean baruch,
        @Parameter(description = "Filter for RPI users") @RequestParam(required = false, defaultValue = "false")
                final boolean rpi,
        @Parameter(description = "Filter for GWC users") @RequestParam(required = false, defaultValue = "false")
                final boolean gwc,
        @Parameter(description = "Filter for SBU users") @RequestParam(required = false, defaultValue = "false")
                final boolean sbu,
        @Parameter(description = "Filter for CCNY users") @RequestParam(required = false, defaultValue = "false")
                final boolean ccny,
        @Parameter(description = "Filter for Columbia users")
                @RequestParam(required = false, defaultValue = "false")
                final boolean columbia,
        @Parameter(description = "Filter for Cornell users") @RequestParam(required = false, defaultValue = "false")
                final boolean cornell,
        @Parameter(description = "Filter for BMCC users") @RequestParam(required = false, defaultValue = "false")
                final boolean bmcc,
        @Parameter(description = "Filter for MHC++ users") @RequestParam(required = false, defaultValue = "false")
                final boolean mhcplusplus,
        @Parameter(description = "Enable global leaderboard index")
                @RequestParam(required = false, defaultValue = "false")
                final boolean globalIndex,
        final HttpServletRequest request) {
    FakeLag.sleep(800);
    final boolean globalIndexNew;
    final LeaderboardFilterOptions options;
    final int parsedPageSize = Math.min(pageSize, MAX_LEADERBOARD_PAGE_SIZE);

    if (filters == null || filters.isEmpty()) {
        globalIndexNew = globalIndex;
        options = LeaderboardFilterOptions.builder()
                .page(page)
                .pageSize(parsedPageSize)
                .query(query)
                .patina(patina)
                .hunter(hunter)
                .nyu(nyu)
                .baruch(baruch)
                .rpi(rpi)
                .gwc(gwc)
                .sbu(sbu)
                .ccny(ccny)
                .columbia(columbia)
                .cornell(cornell)
                .bmcc(bmcc)
                .mhcplusplus(mhcplusplus)
                .build();
    } else {
        globalIndexNew = filters.contains("globalIndex");
        options = LeaderboardFilterOptions.builder()
                .page(page)
                .pageSize(parsedPageSize)
                .query(query)
                .patina(filters.contains("patina"))
                .hunter(filters.contains("hunter"))
                .nyu(filters.contains("nyu"))
                .baruch(filters.contains("baruch"))
                .rpi(filters.contains("rpi"))
                .gwc(filters.contains("gwc"))
                .sbu(filters.contains("sbu"))
                .ccny(filters.contains("ccny"))
                .columbia(filters.contains("columbia"))
                .cornell(filters.contains("cornell"))
                .bmcc(filters.contains("bmcc"))
                .mhcplusplus(filters.contains("mhcplusplus"))
                .build();
    }

@MalihaT111 MalihaT111 force-pushed the 240 branch 2 times, most recently from 02396ae to 97c87d6 Compare April 19, 2026 19:38
@github-actions
Copy link
Copy Markdown
Contributor

Title

240: Create new filters param on previous leaderboard endpoint


PR Type

Enhancement


Description

  • Introduced filters parameter for leaderboard queries.

  • Prioritizes filters set over individual boolean parameters.

  • Updated globalIndex logic based on filters presence.

  • Added comprehensive tests for new filter behavior.


Diagram Walkthrough

flowchart LR
  A[LeaderboardController.getLeaderboardUsersById] --> B{Is 'filters' parameter provided and not empty?};
  B -- Yes --> C[Derive filter options from 'filters' Set];
  B -- No --> D[Derive filter options from individual boolean parameters];
  C --> E[Build LeaderboardFilterOptions];
  D --> E;
  E --> F[Call leaderboardManager.getLeaderboardUsers];
Loading

File Walkthrough

Relevant files
Enhancement
LeaderboardController.java
Add new `filters` parameter to leaderboard endpoint           

src/main/java/org/patinanetwork/codebloom/api/leaderboard/LeaderboardController.java

  • Added a new @RequestParam Set filters to the getLeaderboardUsersById
    method for flexible filtering.
  • Implemented conditional logic to prioritize the filters set for
    building LeaderboardFilterOptions and determining globalIndexNew.
  • If filters is provided and not empty, it parses the set to enable
    specific filters (e.g., nyu, hunter).
  • If filters is null or empty, the method falls back to using the
    existing individual boolean parameters.
+46/-20 
Tests
LeaderboardControllerTest.java
Update and add tests for new leaderboard filters parameter

src/test/java/org/patinanetwork/codebloom/api/leaderboard/LeaderboardControllerTest.java

  • Updated existing test calls to getLeaderboardUsersById to accommodate
    the new filters parameter by passing null.
  • Added usesFiltersSetWhenProvided test to verify that the filters set
    correctly applies specified filters.
  • Included setsGlobalIndexFromFiltersSet test to confirm globalIndex is
    set when present in the filters set.
  • Introduced fallsBackToBooleansWhenFiltersEmpty test to ensure fallback
    to individual boolean parameters when filters is empty.
+94/-0   

@github-actions
Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

240 - Partially compliant

Compliant requirements:

  • Implement filtering for the leaderboard list via a new filters parameter.
  • Backend processes filters for university names (e.g., nyu, hunter) and globalIndex.

Non-compliant requirements:

Requires further human verification:

  • The actual display of the leaderboard list on the admin page.
  • UI functionality of the filters (e.g., dropdown, selection, applying filters).
  • Integration of the frontend with this new backend parameter.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ No major issues detected

@github-actions
Copy link
Copy Markdown
Contributor

Title

240: Create new filters param on previous leaderboard endpoint


PR Type

Enhancement, Tests


Description

  • Add filters parameter for leaderboard.

  • Process filters from new Set<String>.

  • Maintain backward compatibility for filters.

  • Enhance leaderboard API flexibility.


Diagram Walkthrough

flowchart LR
  A[Client Request] --> B{LeaderboardController.getLeaderboardUsersById};
  B -- new filters param --> C{Is 'filters' Set provided and not empty?};
  C -- Yes --> D[Use 'filters' Set to build options];
  C -- No --> E[Fall back to individual boolean params];
  D --> F[Call leaderboardManager.getLeaderboardUsers];
  E --> F;
  F --> G[Return Leaderboard Data];
Loading

File Walkthrough

Relevant files
Enhancement
LeaderboardController.java
Add `filters` parameter and update leaderboard logic         

src/main/java/org/patinanetwork/codebloom/api/leaderboard/LeaderboardController.java

  • Introduced filters parameter as Set for leaderboard endpoint.
  • Implemented logic to prioritize filters over individual boolean
    parameters.
  • Ensured fallback to existing boolean parameters if filters is empty.
  • Updated globalIndex handling based on new filters parameter.
+21/-16 
Tests
LeaderboardControllerTest.java
Add tests for new leaderboard `filters` parameter               

src/test/java/org/patinanetwork/codebloom/api/leaderboard/LeaderboardControllerTest.java

  • Added filters parameter as null to existing test calls for backward
    compatibility.
  • Introduced new tests for filters Set usage, ignoring boolean
    parameters.
  • Verified globalIndex is correctly set from the filters Set.
  • Confirmed fallback to boolean parameters when filters Set is empty.
+94/-0   

@github-actions
Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

240 - Partially compliant

Compliant requirements:

  • Add a new filters parameter to the leaderboard endpoint.
  • The filters parameter should accept a set of strings representing active filters.
  • Implement logic to prioritize the filters set if it is provided and not empty.
  • If the filters parameter is null or empty, the endpoint should fall back to using the existing individual boolean filter parameters.
  • Update the globalIndex logic to also respect the new filters parameter.
  • Ensure existing leaderboard functionality (without the new filters parameter) remains intact.
  • Add comprehensive test coverage for the new filter logic, including fallback scenarios.

Non-compliant requirements:

Requires further human verification:

  • Manual QA in dev and staging (as indicated by unchecked checklist items in the PR description).
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ No major issues detected

@MalihaT111
Copy link
Copy Markdown
Contributor Author

/deploy

1 similar comment
@Arshadul-Monir
Copy link
Copy Markdown
Contributor

/deploy

leaderboard endpoint and fixed leaderboard tests.
@github-actions
Copy link
Copy Markdown
Contributor

Title

240: Create new filters param on previous leaderboard endpoint


PR Type

Enhancement, Tests


Description

  • Add filters parameter to leaderboard endpoint.

  • Dynamically apply filters from Set if provided.

  • Fallback to individual boolean filters.

  • Update tests for new filter logic.


Diagram Walkthrough

flowchart LR
  A[API Request] --> B{Filters parameter provided?};
  B -- Yes --> C{Parse filters Set};
  B -- No --> D{Use individual boolean params};
  C --> E[Build LeaderboardFilterOptions];
  D --> E;
  E --> F[Get Leaderboard Users];
Loading

File Walkthrough

Relevant files
Enhancement
LeaderboardController.java
Add `filters` parameter and conditional filtering logic   

src/main/java/org/patinanetwork/codebloom/api/leaderboard/LeaderboardController.java

  • Added an optional filters @RequestParam of type Set to
    getLeaderboardUsersById.
  • Implemented logic to check if the filters set is active (useFilters).
  • Conditionally sets globalIndex and other boolean filter options in
    LeaderboardFilterOptions based on the filters set or individual
    boolean parameters.
+21/-16 
Tests
LeaderboardControllerTest.java
Update and add tests for new leaderboard filtering             

src/test/java/org/patinanetwork/codebloom/api/leaderboard/LeaderboardControllerTest.java

  • Updated existing test calls to getLeaderboardUsersById to include null
    for the new filters parameter.
  • Added usesFiltersSetWhenProvided test to verify correct filter
    application when a Set is provided.
  • Added setsGlobalIndexFromFiltersSet test to ensure globalIndex is
    correctly derived from the filters set.
  • Added fallsBackToBooleansWhenFiltersEmpty test to confirm fallback to
    individual boolean parameters when the filters set is empty.
+94/-0   

@github-actions
Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

240 - Partially compliant

Compliant requirements:

  • Implement filtering functionality for the leaderboard list (backend API support).
  • Display a screenshot of the new leaderboard list with filters (PR description includes screenshots of the new UI using the API).

Non-compliant requirements:

  • Add a leaderboard list to the admin page (This PR focuses on the backend API for filtering, not the full UI implementation of the admin page itself, though it enables it).

Requires further human verification:

  • Verify the full functionality of the leaderboard list on the admin page in a browser.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ No major issues detected

Comment on lines +145 to 161
final LeaderboardFilterOptions options = LeaderboardFilterOptions.builder()
.page(page)
.pageSize(parsedPageSize)
.query(query)
.patina(patina)
.hunter(hunter)
.nyu(nyu)
.baruch(baruch)
.rpi(rpi)
.gwc(gwc)
.sbu(sbu)
.ccny(ccny)
.columbia(columbia)
.cornell(cornell)
.bmcc(bmcc)
.mhcplusplus(mhcplusplus)
.patina(useFilters ? activeFilters.contains("patina") : patina)
.hunter(useFilters ? activeFilters.contains("hunter") : hunter)
.nyu(useFilters ? activeFilters.contains("nyu") : nyu)
.baruch(useFilters ? activeFilters.contains("baruch") : baruch)
.rpi(useFilters ? activeFilters.contains("rpi") : rpi)
.gwc(useFilters ? activeFilters.contains("gwc") : gwc)
.sbu(useFilters ? activeFilters.contains("sbu") : sbu)
.ccny(useFilters ? activeFilters.contains("ccny") : ccny)
.columbia(useFilters ? activeFilters.contains("columbia") : columbia)
.cornell(useFilters ? activeFilters.contains("cornell") : cornell)
.bmcc(useFilters ? activeFilters.contains("bmcc") : bmcc)
.mhcplusplus(useFilters ? activeFilters.contains("mhcplusplus") : mhcplusplus)
.build();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggestion: The new conditional logic for setting each filter in the LeaderboardFilterOptions builder is highly repetitive. Encapsulating this logic in a private helper method would significantly improve readability and maintainability, especially if more filters are added or the logic changes. This aligns with the "Abstraction / Code Structure" guideline. [general, importance: 7]

Suggested change
final LeaderboardFilterOptions options = LeaderboardFilterOptions.builder()
.page(page)
.pageSize(parsedPageSize)
.query(query)
.patina(patina)
.hunter(hunter)
.nyu(nyu)
.baruch(baruch)
.rpi(rpi)
.gwc(gwc)
.sbu(sbu)
.ccny(ccny)
.columbia(columbia)
.cornell(cornell)
.bmcc(bmcc)
.mhcplusplus(mhcplusplus)
.patina(useFilters ? activeFilters.contains("patina") : patina)
.hunter(useFilters ? activeFilters.contains("hunter") : hunter)
.nyu(useFilters ? activeFilters.contains("nyu") : nyu)
.baruch(useFilters ? activeFilters.contains("baruch") : baruch)
.rpi(useFilters ? activeFilters.contains("rpi") : rpi)
.gwc(useFilters ? activeFilters.contains("gwc") : gwc)
.sbu(useFilters ? activeFilters.contains("sbu") : sbu)
.ccny(useFilters ? activeFilters.contains("ccny") : ccny)
.columbia(useFilters ? activeFilters.contains("columbia") : columbia)
.cornell(useFilters ? activeFilters.contains("cornell") : cornell)
.bmcc(useFilters ? activeFilters.contains("bmcc") : bmcc)
.mhcplusplus(useFilters ? activeFilters.contains("mhcplusplus") : mhcplusplus)
.build();
final LeaderboardFilterOptions options = LeaderboardFilterOptions.builder()
.page(page)
.pageSize(parsedPageSize)
.query(query)
.patina(getFilterValue(useFilters, activeFilters, "patina", patina))
.hunter(getFilterValue(useFilters, activeFilters, "hunter", hunter))
.nyu(getFilterValue(useFilters, activeFilters, "nyu", nyu))
.baruch(getFilterValue(useFilters, activeFilters, "baruch", baruch))
.rpi(getFilterValue(useFilters, activeFilters, "rpi", rpi))
.gwc(getFilterValue(useFilters, activeFilters, "gwc", gwc))
.sbu(getFilterValue(useFilters, activeFilters, "sbu", sbu))
.ccny(getFilterValue(useFilters, activeFilters, "ccny", ccny))
.columbia(getFilterValue(useFilters, activeFilters, "columbia", columbia))
.cornell(getFilterValue(useFilters, activeFilters, "cornell", cornell))
.bmcc(getFilterValue(useFilters, activeFilters, "bmcc", bmcc))
.mhcplusplus(getFilterValue(useFilters, activeFilters, "mhcplusplus", mhcplusplus))
.build();
// Add this private helper method to the LeaderboardController class:
private boolean getFilterValue(boolean useFilters, Set<String> activeFilters, String filterName, boolean defaultValue) {
return useFilters ? activeFilters.contains(filterName) : defaultValue;
}

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