Skip to content

Commit

Permalink
Ensure MilestoneGroup.milestones is sorted correctly (#159)
Browse files Browse the repository at this point in the history 
- sort milestones list by ascending order of `Milestone.order`
- update test data to include ordering that is not the same as the order of ids
  • Loading branch information
@lkeegan @Majapur1
lkeegan authored and Majapur1 committed Nov 14, 2024
1 parent 572a58a commit 8ea4e85
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 97 deletions.
4 changes: 3 additions & 1 deletion mondey_backend/src/mondey_backend/models/milestones.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@ class MilestoneGroup(SQLModel, table=True):
id: int | None = Field(default=None, primary_key=True)
order: int = 0
text: Mapped[dict[str, MilestoneGroupText]] = dict_relationship(key="lang_id")
milestones: Mapped[list[Milestone]] = back_populates("group")
milestones: Mapped[list[Milestone]] = back_populates(
"group", order_by="asc(Milestone.order)"
)


class MilestoneGroupPublic(SQLModel):
Expand Down
4 changes: 2 additions & 2 deletions mondey_backend/src/mondey_backend/models/utils.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@
from sqlmodel import Relationship


def back_populates(name: str):
def back_populates(name: str, **kwargs):
# Workaround for "please state the generic argument using an annotation" sqlalchemy error
# https://github.com/fastapi/sqlmodel/discussions/771#discussioncomment-10326074
return Relationship(sa_relationship=relationship(back_populates=name))
return Relationship(sa_relationship=relationship(back_populates=name, **kwargs))


def dict_relationship(key: str):
Expand Down
184 changes: 95 additions & 89 deletions mondey_backend/tests/conftest.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,11 @@ def session(children: list[dict]):
)
for milestone_id in [1, 2, 3]:
session.add(
Milestone(order=0, group_id=1, expected_age_months=milestone_id * 6)
Milestone(
order=4 - milestone_id,
group_id=1,
expected_age_months=milestone_id * 6,
)
)
for lang_id in lang_ids:
lbl = f"m{milestone_id}_{lang_id}"
Expand All @@ -155,7 +159,9 @@ def session(children: list[dict]):
)
for milestone_id in [4, 5]:
session.add(
Milestone(order=0, group_id=2, expected_age_months=milestone_id * 6)
Milestone(
order=milestone_id, group_id=2, expected_age_months=milestone_id * 6
)
)
for lang_id in lang_ids:
lbl = f"m{milestone_id}_{lang_id}"
Expand Down Expand Up @@ -697,32 +703,29 @@ def milestone_group1():
},
"milestones": [
{
"id": 1,
"expected_age_months": 6,
"id": 3,
"expected_age_months": 18,
"text": {
"de": {
"title": "m1_de_t",
"desc": "m1_de_d",
"obs": "m1_de_o",
"help": "m1_de_h",
"title": "m3_de_t",
"desc": "m3_de_d",
"obs": "m3_de_o",
"help": "m3_de_h",
},
"en": {
"title": "m1_en_t",
"desc": "m1_en_d",
"obs": "m1_en_o",
"help": "m1_en_h",
"title": "m3_en_t",
"desc": "m3_en_d",
"obs": "m3_en_o",
"help": "m3_en_h",
},
"fr": {
"title": "m1_fr_t",
"desc": "m1_fr_d",
"obs": "m1_fr_o",
"help": "m1_fr_h",
"title": "m3_fr_t",
"desc": "m3_fr_d",
"obs": "m3_fr_o",
"help": "m3_fr_h",
},
},
"images": [
{"filename": "m1.jpg", "approved": True},
{"filename": "m2.jpg", "approved": True},
],
"images": [],
},
{
"id": 2,
Expand Down Expand Up @@ -750,29 +753,32 @@ def milestone_group1():
"images": [{"filename": "m3.jpg", "approved": True}],
},
{
"id": 3,
"expected_age_months": 18,
"id": 1,
"expected_age_months": 6,
"text": {
"de": {
"title": "m3_de_t",
"desc": "m3_de_d",
"obs": "m3_de_o",
"help": "m3_de_h",
"title": "m1_de_t",
"desc": "m1_de_d",
"obs": "m1_de_o",
"help": "m1_de_h",
},
"en": {
"title": "m3_en_t",
"desc": "m3_en_d",
"obs": "m3_en_o",
"help": "m3_en_h",
"title": "m1_en_t",
"desc": "m1_en_d",
"obs": "m1_en_o",
"help": "m1_en_h",
},
"fr": {
"title": "m3_fr_t",
"desc": "m3_fr_d",
"obs": "m3_fr_o",
"help": "m3_fr_h",
"title": "m1_fr_t",
"desc": "m1_fr_d",
"obs": "m1_fr_o",
"help": "m1_fr_h",
},
},
"images": [],
"images": [
{"filename": "m1.jpg", "approved": True},
{"filename": "m2.jpg", "approved": True},
],
},
],
}
Expand Down Expand Up @@ -806,53 +812,40 @@ def milestone_group_admin1():
"milestones": [
{
"group_id": 1,
"order": 0,
"id": 1,
"expected_age_months": 6,
"images": [
{
"id": 1,
"milestone_id": 1,
"filename": "m1.jpg",
"approved": True,
},
{
"id": 2,
"milestone_id": 1,
"filename": "m2.jpg",
"approved": True,
},
],
"order": 1,
"id": 3,
"expected_age_months": 18,
"images": [],
"text": {
"de": {
"obs": "m1_de_o",
"help": "m1_de_h",
"title": "m1_de_t",
"desc": "m1_de_d",
"milestone_id": 1,
"obs": "m3_de_o",
"help": "m3_de_h",
"title": "m3_de_t",
"desc": "m3_de_d",
"milestone_id": 3,
"lang_id": "de",
},
"en": {
"obs": "m1_en_o",
"help": "m1_en_h",
"title": "m1_en_t",
"desc": "m1_en_d",
"milestone_id": 1,
"obs": "m3_en_o",
"help": "m3_en_h",
"title": "m3_en_t",
"desc": "m3_en_d",
"milestone_id": 3,
"lang_id": "en",
},
"fr": {
"obs": "m1_fr_o",
"help": "m1_fr_h",
"title": "m1_fr_t",
"desc": "m1_fr_d",
"milestone_id": 1,
"obs": "m3_fr_o",
"help": "m3_fr_h",
"title": "m3_fr_t",
"desc": "m3_fr_d",
"milestone_id": 3,
"lang_id": "fr",
},
},
},
{
"group_id": 1,
"order": 0,
"order": 2,
"id": 2,
"expected_age_months": 12,
"images": [
Expand Down Expand Up @@ -892,33 +885,46 @@ def milestone_group_admin1():
},
{
"group_id": 1,
"order": 0,
"id": 3,
"expected_age_months": 18,
"images": [],
"order": 3,
"id": 1,
"expected_age_months": 6,
"images": [
{
"id": 1,
"milestone_id": 1,
"filename": "m1.jpg",
"approved": True,
},
{
"id": 2,
"milestone_id": 1,
"filename": "m2.jpg",
"approved": True,
},
],
"text": {
"de": {
"obs": "m3_de_o",
"help": "m3_de_h",
"title": "m3_de_t",
"desc": "m3_de_d",
"milestone_id": 3,
"obs": "m1_de_o",
"help": "m1_de_h",
"title": "m1_de_t",
"desc": "m1_de_d",
"milestone_id": 1,
"lang_id": "de",
},
"en": {
"obs": "m3_en_o",
"help": "m3_en_h",
"title": "m3_en_t",
"desc": "m3_en_d",
"milestone_id": 3,
"obs": "m1_en_o",
"help": "m1_en_h",
"title": "m1_en_t",
"desc": "m1_en_d",
"milestone_id": 1,
"lang_id": "en",
},
"fr": {
"obs": "m3_fr_o",
"help": "m3_fr_h",
"title": "m3_fr_t",
"desc": "m3_fr_d",
"milestone_id": 3,
"obs": "m1_fr_o",
"help": "m1_fr_h",
"title": "m1_fr_t",
"desc": "m1_fr_d",
"milestone_id": 1,
"lang_id": "fr",
},
},
Expand Down Expand Up @@ -1019,7 +1025,7 @@ def milestone_group_admin2():
"milestones": [
{
"group_id": 2,
"order": 0,
"order": 4,
"id": 4,
"expected_age_months": 24,
"images": [],
Expand Down Expand Up @@ -1052,7 +1058,7 @@ def milestone_group_admin2():
},
{
"group_id": 2,
"order": 0,
"order": 5,
"id": 5,
"expected_age_months": 30,
"images": [],
Expand Down
10 changes: 5 additions & 5 deletions mondey_backend/tests/routers/test_milestones.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ def test_get_milestone_groups_child1(
assert len(response.json()) == 2
# child 1 age is ~9 months old, so no milestones from group2
milestone_group2["milestones"] = []
# and only the first two milestones (6m, 12m) from group1:
milestone_group1["milestones"] = milestone_group1["milestones"][0:2]
# and only last two milestones (6m, 12m) from group1:
milestone_group1["milestones"] = milestone_group1["milestones"][1:]
assert response.json() == [milestone_group2, milestone_group1]


Expand All @@ -31,9 +31,9 @@ def test_get_milestone_groups_child2(
response = user_client.get("/milestone-groups/2")
assert response.status_code == 200
assert len(response.json()) == 2
# child 2 age is 20 months old, so last milestone from group1 (18m):
milestone_group1["milestones"] = milestone_group1["milestones"][-1:]
# and first last milestone from group2 (24m):
# child 2 age is 20 months old, so first milestone from group1 (18m):
milestone_group1["milestones"] = milestone_group1["milestones"][0:1]
# and first milestone from group2 (24m):
milestone_group2["milestones"] = milestone_group2["milestones"][0:1]
assert response.json() == [milestone_group2, milestone_group1]

Expand Down

0 comments on commit 8ea4e85

Please sign in to comment.