Skip to content

Fix/issue 27 server side sorting - #39

Open
saesarandom wants to merge 2 commits into
coleestrin:mainfrom
saesarandom:fix/issue-27-server-side-sorting
Open

Fix/issue 27 server side sorting#39
saesarandom wants to merge 2 commits into
coleestrin:mainfrom
saesarandom:fix/issue-27-server-side-sorting

Conversation

@saesarandom

Copy link
Copy Markdown
Contributor

Summary of Changes:

  • Database: Added dynamic ORDER BY handling (level, name, life, mana) in getFilteredCharacters() prior to pagination LIMIT/OFFSET.
  • API Route: Extracted sortBy & sortOrder query params in /api/characters.
  • Frontend API: Passed sort parameters through charactersAPI.getCharacters().
  • CharacterTable: Configured manualSorting: true, sorting state, and onSortingChange handler to refetch page 1 with global sorting when column headers are clicked.

@ChaseBianchi

Copy link
Copy Markdown
Contributor

rebase this on the latest from main

@saesarandom
saesarandom force-pushed the fix/issue-27-server-side-sorting branch from 02058b6 to 335c647 Compare July 27, 2026 10:45
// If we have initial characters and this is not a pagination request,
const activeSort = sorting[0];
const sortBy = activeSort?.id;
const sortOrder = activeSort?.desc ? "desc" : "asc";

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.

Let’s make the table’s default sort explicit. With the current code, no active sort produces sortBy=undefined but sortOrder="asc". The initial page is level-descending, while later pagination requests become level-ascending. Defaulting to level/desc keeps every page consistently ordered:

const sortBy = activeSort?.id ?? "level";
const sortOrder = activeSort
  ? activeSort.desc
    ? "desc"
    : "asc"
  : "desc";

orderByClause = `(C.full_response_json->'character'->>'mana')::int ${orderDirection}, C.character_db_id DESC`;
} else if (filter.sortBy === "level") {
orderByClause = `C.level ${orderDirection}, C.character_db_id DESC`;
}

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.

CharacterTable can send sortBy="highestSkLevel", but none of the branches above handle it, so clicking that column currently falls back to sorting by character level. Let's add this branch before the ordering block ends:

} else if (filter.sortBy === "highestSkLevel") {
  orderByClause = `
    (C.full_response_json->'realSkills'->0->>'level')::int
      ${orderDirection}
      NULLS LAST,
    C.character_db_id DESC
  `;
}

columns,
data: tableDisplayData,
manualPagination: true,
manualSorting: true,

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.

Mantine React Table enables multi-column sorting by default, but this implementation only sends sorting[0] to the API. Shift-clicking another sortable column therefore displays multiple sort indicators while the server ignores every key after the first. Please set enableMultiSort: false, or extend the API to support all selected keys.

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