diff --git a/backend/api/v1/tasks/viewsets/board_groups.py b/backend/api/v1/tasks/viewsets/board_groups.py index c33e30dc..663cae45 100644 --- a/backend/api/v1/tasks/viewsets/board_groups.py +++ b/backend/api/v1/tasks/viewsets/board_groups.py @@ -20,6 +20,7 @@ from tasks.group_ranking import ( allocate_group_tail_rank_keys, rank_between, rebalance_group_memberships, + sort_group_memberships_by_rank, visible_group_position_map, ) from tasks.models import ( @@ -288,10 +289,10 @@ class TaskBoardGroupViewSet(viewsets.ModelViewSet): with transaction.atomic(): locked_group = TaskBoardGroup.objects.select_for_update().get(pk=group.pk) - memberships = list( - TaskBoardGroupMembership.objects.select_for_update() - .filter(group=locked_group) - .order_by("rank_key", "id") + memberships = sort_group_memberships_by_rank( + TaskBoardGroupMembership.objects.select_for_update().filter( + group=locked_group + ) ) memberships_by_board = { membership.board_id: membership for membership in memberships diff --git a/backend/tasks/group_ranking.py b/backend/tasks/group_ranking.py index 945e4a8a..f0e5be00 100644 --- a/backend/tasks/group_ranking.py +++ b/backend/tasks/group_ranking.py @@ -43,6 +43,16 @@ def decode_rank(rank_key: str) -> int: return value +def sort_group_memberships_by_rank( + memberships: Sequence[TaskBoardGroupMembership] | QuerySet, +) -> list[TaskBoardGroupMembership]: + """Sort ranks independently from the database's text collation.""" + return sorted( + memberships, + key=lambda membership: (decode_rank(membership.rank_key), membership.id), + ) + + def rank_between(lower_key: str | None, upper_key: str | None) -> str: lower_value = decode_rank(lower_key) if lower_key is not None else 0 upper_value = decode_rank(upper_key) if upper_key is not None else RANK_MAX_VALUE @@ -76,20 +86,33 @@ def rebalance_group_memberships( group: TaskBoardGroup, memberships: Sequence[TaskBoardGroupMembership] | QuerySet | None = None, ) -> list[TaskBoardGroupMembership]: - ordered_memberships = list( + ordered_memberships = sort_group_memberships_by_rank( memberships if memberships is not None - else TaskBoardGroupMembership.objects.filter(group=group).order_by( - "rank_key", "id" - ) + else TaskBoardGroupMembership.objects.filter(group=group) ) - for membership, rank_key in zip( - ordered_memberships, - evenly_spaced_rank_keys(len(ordered_memberships)), - strict=True, - ): - membership.rank_key = rank_key + final_rank_keys = evenly_spaced_rank_keys(len(ordered_memberships)) if ordered_memberships: + # Rank keys are unique inside a group. Updating directly from the old + # order to the new one can make PostgreSQL observe a transient + # duplicate (A receives B's key before B releases it). Move every row + # into a disjoint temporary namespace first, then assign final keys. + for membership in ordered_memberships: + temporary_suffix = format(membership.pk, "031x") + if len(temporary_suffix) > RANK_WIDTH - 1: + raise ValueError("Membership id is too large for a temporary rank key.") + membership.rank_key = f"~{temporary_suffix}" + TaskBoardGroupMembership.objects.bulk_update( + ordered_memberships, + ["rank_key"], + ) + + for membership, rank_key in zip( + ordered_memberships, + final_rank_keys, + strict=True, + ): + membership.rank_key = rank_key TaskBoardGroupMembership.objects.bulk_update( ordered_memberships, ["rank_key"], @@ -103,12 +126,12 @@ def allocate_group_tail_rank_keys( ) -> list[str]: if count <= 0: return [] - last_key = ( - TaskBoardGroupMembership.objects.filter(group=group) - .order_by("-rank_key", "-id") - .values_list("rank_key", flat=True) - .first() + rank_keys = list( + TaskBoardGroupMembership.objects.filter(group=group).values_list( + "rank_key", flat=True + ) ) + last_key = max(rank_keys, key=decode_rank, default=None) try: return allocate_rank_keys_after(last_key, count) except RankSpaceExhausted: @@ -130,19 +153,26 @@ def visible_group_position_map( memberships = ( TaskBoardGroupMembership.objects.filter( board__in=accessible_boards, - ).filter( + ) + .filter( Q(group__user=user) | Q(group__is_shared=True), ) .exclude(group__hidden_by=user) - .order_by("group_id", "rank_key", "id") ) if group_ids is not None: memberships = memberships.filter(group_id__in=group_ids) + membership_rows = list( + memberships.values_list("group_id", "board_id", "rank_key", "id") + ) + membership_rows.sort( + key=lambda row: (row[0], decode_rank(row[2]), row[3]), + ) + result: dict[tuple[int, int], int] = {} current_group_id = None position = 0 - for group_id, board_id in memberships.values_list("group_id", "board_id"): + for group_id, board_id, _rank_key, _membership_id in membership_rows: if group_id != current_group_id: current_group_id = group_id position = 0 diff --git a/backend/tests/api/v1/tasks/test_tasks_api.py b/backend/tests/api/v1/tasks/test_tasks_api.py index 66861f21..68b676c9 100644 --- a/backend/tests/api/v1/tasks/test_tasks_api.py +++ b/backend/tests/api/v1/tasks/test_tasks_api.py @@ -45,7 +45,13 @@ from tasks.models import ( TaskLinkedObjectKind, TaskUserSettings, ) -from tasks.group_ranking import evenly_spaced_rank_keys, rank_between +from tasks.group_ranking import ( + decode_rank, + encode_rank, + evenly_spaced_rank_keys, + rank_between, + sort_group_memberships_by_rank, +) from tasks.realtime import get_task_board_recipient_ids from api.v1.tasks.views import TaskViewSet @@ -661,6 +667,30 @@ def test_rank_keys_support_one_hundred_insertions_between_neighbours(): assert all(lower_key < rank_key for rank_key in generated_keys) +def test_group_rank_sorting_does_not_depend_on_database_text_order(user): + boards = [ + TaskBoard.objects.create(name=f"Порядок ключей {index}", created_by=user) + for index in range(1, 4) + ] + group = TaskBoardGroup.objects.create(name="Числовой порядок", user=user) + memberships = [ + TaskBoardGroupMembership.objects.create( + group=group, + board=board, + rank_key=encode_rank(rank_value), + ) + for board, rank_value in zip(boards, [62**20, 61, 62], strict=True) + ] + + ordered = sort_group_memberships_by_rank(reversed(memberships)) + + assert [membership.board_id for membership in ordered] == [ + boards[1].id, + boards[2].id, + boards[0].id, + ] + + def test_board_group_boards_are_replaced_in_one_bulk_request( api_client, user, @@ -812,6 +842,127 @@ def test_board_can_be_reordered_inside_one_group_by_changing_only_its_rank( ).rank_key == secondary_rank_key +def test_board_reorder_rebalances_rank_keys_without_transient_duplicates( + api_client, + user, +): + boards = [ + TaskBoard.objects.create(name=f"Плотная доска {index}", created_by=user) + for index in range(1, 5) + ] + group = TaskBoardGroup.objects.create(name="Плотные приоритеты", user=user) + first_key, second_key, third_key, _fourth_key = evenly_spaced_rank_keys(4) + dense_key = encode_rank(decode_rank(first_key) + 1) + TaskBoardGroupMembership.objects.bulk_create( + [ + TaskBoardGroupMembership( + group=group, + board=boards[0], + rank_key=first_key, + ), + TaskBoardGroupMembership( + group=group, + board=boards[3], + rank_key=dense_key, + ), + TaskBoardGroupMembership( + group=group, + board=boards[1], + rank_key=second_key, + ), + TaskBoardGroupMembership( + group=group, + board=boards[2], + rank_key=third_key, + ), + ] + ) + api_client.force_authenticate(user=user) + + response = api_client.post( + reverse( + "api:v1:tasks:task-board-group-reorder-board", + kwargs={"pk": group.id}, + ), + { + "board": boards[1].id, + "previous_board": boards[0].id, + "next_board": boards[3].id, + }, + format="json", + ) + + assert response.status_code == status.HTTP_200_OK + assert [ + membership.board_id + for membership in sort_group_memberships_by_rank( + TaskBoardGroupMembership.objects.filter(group=group) + ) + ] == [boards[0].id, boards[1].id, boards[3].id, boards[2].id] + assert response.data["positions"] == { + str(boards[0].id): 1, + str(boards[1].id): 2, + str(boards[3].id): 3, + str(boards[2].id): 4, + } + + +def test_every_board_can_be_moved_to_every_visible_position(api_client, user): + boards = [ + TaskBoard.objects.create(name=f"Свободная доска {index}", created_by=user) + for index in range(1, 6) + ] + group = TaskBoardGroup.objects.create(name="Свободный порядок", user=user) + TaskBoardGroupMembership.objects.bulk_create( + [ + TaskBoardGroupMembership(group=group, board=board, rank_key=rank_key) + for board, rank_key in zip( + boards, + evenly_spaced_rank_keys(len(boards)), + strict=True, + ) + ] + ) + api_client.force_authenticate(user=user) + reorder_url = reverse( + "api:v1:tasks:task-board-group-reorder-board", + kwargs={"pk": group.id}, + ) + expected_order = [board.id for board in boards] + + for target_index in range(len(boards)): + for board_id in [board.id for board in boards]: + remaining_order = [item for item in expected_order if item != board_id] + next_order = [*remaining_order] + next_order.insert(target_index, board_id) + if next_order == expected_order: + continue + + moved_index = next_order.index(board_id) + response = api_client.post( + reorder_url, + { + "board": board_id, + "previous_board": next_order[moved_index - 1] + if moved_index > 0 + else None, + "next_board": next_order[moved_index + 1] + if moved_index + 1 < len(next_order) + else None, + }, + format="json", + ) + + assert response.status_code == status.HTTP_200_OK + expected_order = next_order + assert [ + membership.board_id + for membership in sort_group_memberships_by_rank( + TaskBoardGroupMembership.objects.filter(group=group) + ) + ] == expected_order + + def test_board_reorder_rejects_stale_visible_neighbours(api_client, user): boards = [ TaskBoard.objects.create(name=f"Сосед {index}", created_by=user) diff --git a/frontend/src/components/tasks/TaskBoardNavigator.tsx b/frontend/src/components/tasks/TaskBoardNavigator.tsx index 9ce7d7f1..1c3f7b17 100644 --- a/frontend/src/components/tasks/TaskBoardNavigator.tsx +++ b/frontend/src/components/tasks/TaskBoardNavigator.tsx @@ -5,8 +5,10 @@ import { MouseSensor, TouchSensor, closestCenter, + pointerWithin, useSensor, useSensors, + type CollisionDetection, type DragEndEvent, } from "@dnd-kit/core"; import { @@ -86,6 +88,11 @@ const BOARD_SORT_OPTIONS = [ const NAVIGATOR_COLLAPSED_STORAGE_PREFIX = "tasks.board-navigator-collapsed"; +const boardPointerCollisionDetection: CollisionDetection = (args) => { + const pointerCollisions = pointerWithin(args); + return pointerCollisions.length > 0 ? pointerCollisions : closestCenter(args); +}; + function boardGroupIds(board: TaskBoardSummary): number[] { return board.group_ids || (board.group_id ? [board.group_id] : []); } @@ -1028,7 +1035,7 @@ export default function TaskBoardNavigator({ {visibleBoards.length > 0 ? ( void handleBoardDragEnd(event)} >