Skip to content

Commit

Permalink
Fix optimisitic update when deleting moves
Browse files Browse the repository at this point in the history
  • Loading branch information
LucasPickering committed Jul 30, 2023
1 parent 6e317d4 commit f0348c2
Show file tree
Hide file tree
Showing 9 changed files with 418 additions and 279 deletions.
4 changes: 4 additions & 0 deletions api/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,8 @@ src = ["src/"]

[tool.pytest.ini_options]
DJANGO_SETTINGS_MODULE = "beta_spray.settings.settings_dev"
filterwarnings = [
# These are from internal strawberry code
"ignore:Argument name-based matching*:DeprecationWarning",
]
python_files = ["test_*.py"]
10 changes: 9 additions & 1 deletion api/src/core/tests/factories.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,16 @@ class Meta:
class BetaFactory(DjangoModelFactory):
class Meta:
model = Beta
skip_postgeneration_save = True

problem = factory.SubFactory(ProblemFactory)
owner = factory.SubFactory(UserFactory)
name = Faker("name")
moves = factory.RelatedFactoryList(
"core.tests.factories.BetaMoveFactory",
factory_related_name="beta",
size=5,
)


class BetaMoveFactory(DjangoModelFactory):
Expand All @@ -95,7 +101,8 @@ class Meta:
class Params:
is_free = Faker("pybool")

beta = factory.SubFactory(BetaFactory)
beta = factory.SubFactory(BetaFactory, moves=[])
order = factory.Sequence(lambda n: n)
hold = factory.Maybe(
"is_free",
yes_declaration=None,
Expand All @@ -110,6 +117,7 @@ class Params:

# We have to register at the bottom, to avoid circular import issues
register(UserFactory)
register(UserFactory, _name="other_user")
register(UserProfileFactory)
register(BoulderFactory)
register(ProblemFactory)
Expand Down
133 changes: 133 additions & 0 deletions api/src/core/tests/schema/mutation/test_mutate_beta_move.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
import pytest
from pytest_factoryboy import LazyFixture
from strawberry import relay
from strawberry.django.context import StrawberryDjangoContext

from core.models import BetaMove
from core.schema import schema
from core.schema.query import BetaMoveNode
from core.tests.schema.conftest import assert_graphql_result

pytestmark = pytest.mark.django_db

create_beta_move_mutation = """
mutation($input: CreateBetaMoveInput!) {
createBetaMove(input: $input) {
id
order
bodyPart
isStart
annotation
target
permissions {
canEdit
canDelete
}
beta {
moves {
edges {
node {
id
order
}
}
}
}
}
}
"""

update_beta_move_mutation = """
mutation($input: UpdateBetaMoveInput!) {
updateBetaMove(input: $input) {
id
order
bodyPart
isStart
annotation
target {
__typename
}
permissions {
canEdit
canDelete
}
beta {
moves {
edges {
node {
id
order
}
}
}
}
}
}
"""

delete_beta_move_mutation = """
mutation($input: NodeInput!) {
deleteBetaMove(input: $input) {
id
beta {
moves {
edges {
node {
id
order
}
}
}
}
}
}
"""


def test_delete_beta_move(
context: StrawberryDjangoContext, beta_move: BetaMove
) -> None:
beta_move_id = relay.to_base64(BetaMoveNode, beta_move.id)
assert_graphql_result(
schema.execute_sync(
delete_beta_move_mutation,
context_value=context,
variable_values={"input": {"id": beta_move_id}},
),
# This returns the *old* value
{
"deleteBetaMove": {
"id": beta_move_id,
"beta": {
"moves": {
"edges": [
{
"node": {
"id": beta_move_id,
"order": beta_move.order,
}
}
]
}
},
}
},
)
assert BetaMove.objects.filter(id=beta_move.id).count() == 0


@pytest.mark.parametrize("beta__owner", [LazyFixture("other_user")])
def test_delete_beta_move_no_permission(
context: StrawberryDjangoContext, beta_move: BetaMove
) -> None:
beta_move_id = relay.to_base64(BetaMoveNode, beta_move.id)
assert_graphql_result(
schema.execute_sync(
delete_beta_move_mutation,
context_value=context,
variable_values={"input": {"id": beta_move_id}},
),
None,
["You don't have permission"],
)
11 changes: 4 additions & 7 deletions api/src/core/tests/schema/mutation/test_mutate_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

from core.schema import schema
from core.schema.query import UserNode
from core.tests.factories import UserFactory
from core.tests.schema.conftest import assert_graphql_result

pytestmark = pytest.mark.django_db
Expand Down Expand Up @@ -50,10 +49,7 @@ def test_update_user_success(
("aa", "Invalid username"),
("bad@", "Invalid username"),
("emoji bad 💀", "Invalid username"),
(
"too_long_too_long_too_long_too_long",
"Invalid username",
),
("too_long_too_long_too_long_too_long", "Invalid username"),
],
)
def test_update_user_invalid_username(
Expand All @@ -71,8 +67,9 @@ def test_update_user_invalid_username(
)


def test_update_user_someone_else(context: StrawberryDjangoContext) -> None:
other_user = UserFactory()
def test_update_user_someone_else(
context: StrawberryDjangoContext, other_user: User
) -> None:
other_user_id = relay.to_base64(UserNode, other_user.id)
assert_graphql_result(
schema.execute_sync(
Expand Down
97 changes: 13 additions & 84 deletions ui/src/components/Editor/EditorControls/BetaMoveList.tsx
Original file line number Diff line number Diff line change
@@ -1,21 +1,18 @@
import { isDefined, findNodeIndex, moveArrayElement } from "util/func";
import useMutation from "util/useMutation";
import { List, Typography } from "@mui/material";
import MutationErrorSnackbar from "components/common/MutationErrorSnackbar";
import { useState, useMemo, useEffect } from "react";
import { graphql, useFragment } from "react-relay";
import { deleteBetaMoveLocal, reorderBetaMoveLocal } from "../util/moves";
import {
useStance,
useStickFigureColor as useStanceColor,
useStanceControls,
} from "../util/stance";
import { DragItem } from "../util/dnd";
import useBetaMoveMutations from "../util/useBetaMoveMutations";
import BetaDetailsDragLayer from "./BetaDetailsDragLayer";
import BetaMoveListItemSmart from "./BetaMoveListItemSmart";
import { BetaMoveList_betaNode$key } from "./__generated__/BetaMoveList_betaNode.graphql";
import { BetaMoveList_deleteBetaMoveMutation } from "./__generated__/BetaMoveList_deleteBetaMoveMutation.graphql";
import { BetaMoveList_updateBetaMoveMutation } from "./__generated__/BetaMoveList_updateBetaMoveMutation.graphql";

interface Props {
betaKey: BetaMoveList_betaNode$key;
Expand All @@ -29,13 +26,15 @@ const BetaMoveList: React.FC<Props> = ({ betaKey }) => {
const beta = useFragment(
graphql`
fragment BetaMoveList_betaNode on BetaNode {
...useBetaMoveMutations_betaNode
id
permissions {
canEdit
}
moves {
...BetaDetailsDragLayer_betaMoveNodeConnection
...stance_betaMoveNodeConnection
__id
edges {
node {
id
Expand All @@ -51,46 +50,10 @@ const BetaMoveList: React.FC<Props> = ({ betaKey }) => {
betaKey
);

const { commit: updateBetaMove, state: updateState } =
useMutation<BetaMoveList_updateBetaMoveMutation>(graphql`
mutation BetaMoveList_updateBetaMoveMutation($input: UpdateBetaMoveInput!)
@raw_response_type {
updateBetaMove(input: $input) {
beta {
# Refetch all moves to get the new ordering
moves {
edges {
node {
id
order
isStart
}
}
}
}
}
}
`);
const { commit: deleteBetaMove, state: deleteState } =
useMutation<BetaMoveList_deleteBetaMoveMutation>(graphql`
mutation BetaMoveList_deleteBetaMoveMutation($input: NodeInput!)
@raw_response_type {
deleteBetaMove(input: $input) {
beta {
# Refetch all moves to get the new ordering
moves {
edges {
node {
id
order
isStart
}
}
}
}
}
}
`);
const {
reorder: { callback: reorderBetaMove, state: reorderState },
delete: { callback: deleteBetaMove, state: deleteState },
} = useBetaMoveMutations(beta);

// When reordering moves, we need to track temporary state of where the
// dragged move is. We'll use this to reorder the moves locally. Once the
Expand Down Expand Up @@ -140,46 +103,10 @@ const BetaMoveList: React.FC<Props> = ({ betaKey }) => {
// convert now. The API will take care of sliding the
// other moves up/down to fit this one in
const newOrder = item.index + 1;
updateBetaMove({
variables: {
input: {
id: item.betaMoveId,
order: newOrder,
},
},
optimisticResponse: {
updateBetaMove: {
id: item.betaMoveId,
beta: {
id: beta.id,
moves: reorderBetaMoveLocal(
beta.moves,
item.betaMoveId,
newOrder
),
},
},
},
});
reorderBetaMove({ betaMoveId: item.betaMoveId, newOrder });
}
setDraggingMove(undefined);
};
const onDelete = (betaMoveId: string): void => {
deleteBetaMove({
variables: {
input: { id: betaMoveId },
},
optimisticResponse: {
deleteBetaMove: {
id: betaMoveId,
beta: {
id: beta.id,
moves: deleteBetaMoveLocal(beta.moves, betaMoveId),
},
},
},
});
};

const {
permissions: { canEdit },
Expand Down Expand Up @@ -211,14 +138,16 @@ const BetaMoveList: React.FC<Props> = ({ betaKey }) => {
// hide its drag handle
onReorder={canEdit ? onReorder : undefined}
onDrop={canEdit ? onDrop : undefined}
onDelete={canEdit ? () => onDelete(node.id) : undefined}
onDelete={
canEdit ? () => deleteBetaMove({ betaMoveId: node.id }) : undefined
}
/>
))}

<BetaDetailsDragLayer betaMoveConnectionKey={beta.moves} />
<MutationErrorSnackbar
message="Error updating move"
state={updateState}
message="Error reordering move"
state={reorderState}
/>
<MutationErrorSnackbar
message="Error deleting move"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ const BetaEditor: React.FC<Props> = ({ betaKey }) => {
}
...useBetaMoveMutations_betaNode
moves {
__id
edges {
node {
id
Expand Down
Loading

0 comments on commit f0348c2

Please sign in to comment.