Bug fix: Orders could not be interacted with after creation.

[jakekarnes42]

Previously, order items were incorrectly mapped to menu items. You could call successfully call the "Get Orders" API while the orders were empty, but once an order had been created, subsequent calls to that API would fail. Similarly, valid requests to the "Get Order" API would fail due to the incorrect mapping.

This update ensures that orders can be correctly created (including with multiple order items) and then retrieved with the "Get Orders" and "Get Order" APIs.

[theowni]

Hi @jakekarnes42, thank you for the PR!

Could you also create simple unit tests to presents that your changes work well? Currently, I would need to validate these changes manually or write unit tests by myself which I could do tomorrow.

[jakekarnes42]

Hi @theowni,

I've added tests as requested in my latest commit. Specifically, I've updated the existing tests to include OrderItems within the Order. Previously, the tests were creating empty orders and this didn't encounter the bug. I've also add a couple tests to ensure that the code handles Orders with multiple OrderItems, and that these can be retrieved.

There shouldn't be any database migrations necessary since the DB schema isn't changing. We're only correcting how the models are relating to each other. Just to be sure, I ran docker compose run web alembic revision --autogenerate -m 'Fixing Order Item mapping' and here was the output file:

"""Fixing Order Item mapping

Revision ID: eda2ae07dac2
Revises: e2470973db23
Create Date: 2024-05-06 14:53:06.984746

"""
from typing import Sequence, Union

from alembic import op
import sqlalchemy as sa

# revision identifiers, used by Alembic.
revision: str = 'eda2ae07dac2'
down_revision: Union[str, None] = 'e2470973db23'
branch_labels: Union[str, Sequence[str], None] = None
depends_on: Union[str, Sequence[str], None] = None

def upgrade() -> None:
    # ### commands auto generated by Alembic - please adjust! ###
    pass
    # ### end Alembic commands ###

def downgrade() -> None:
    # ### commands auto generated by Alembic - please adjust! ###
    pass
    # ### end Alembic commands ###

If I'm reading that correctly, both the upgrade and downgrade functions are no-ops, so I didn't include that migration file.

[theowni]

hey @jakekarnes42, there are some formatting issues. Could you fix them, please?

These checks are a part of pre-commit setup. The details are in the pre-commit build log.

I double checked the migration and it's fine to not include this generated file ;)

[theowni]

Hi @jakekarnes42, let me know if have any issues issues with fixing these formatting issues. I tried to do it for you but changes are in your forked repository and can't do this unfortunately. I'd need to create a new PR.

I would really like to see this fix merged to the main repo! You did a great job!