Fix board rank ordering and drag collision handling
This commit is contained in:
parent
2e6518bf4b
commit
3c50407b0f
@ -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
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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 ? (
|
||||
<DndContext
|
||||
sensors={boardReorderSensors}
|
||||
collisionDetection={closestCenter}
|
||||
collisionDetection={boardPointerCollisionDetection}
|
||||
onDragEnd={(event) => void handleBoardDragEnd(event)}
|
||||
>
|
||||
<SortableContext
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user