tuttle-dev / tuttle

🪰 Tuttle - time and money management for freelancers
GNU General Public License v3.0
63 stars 12 forks source link

Refactoring: Remove explicit Id handling for model entities #120

Closed clstaudt closed 1 year ago

clstaudt commented 1 year ago

Currently it seems that the app code handles model object ids explicitly:

    def update_user(
        self,
        user: User,
        title: str,
        name: str,
        email: str,
        phone: str,
        street: str,
        street_num: str,
        postal_code: str,
        city: str,
        country: str,
    ) -> IntentResult:
        address_id = user.address_id if user.address_id is not None else 123
        add = Address(
            id=address_id,
            street=street,
            number=street_num,
            postal_code=postal_code,
            city=city,
            country=country,
        )
        user = User(
            id=user.id,
            name=name,
            subtitle=title,
            email=email,
            phone_number=phone,
            address_id=add.id,
            address=add,
        )
        self._dummy_user = user
        return IntentResult(was_intent_successful=True, data=self._dummy_user)

SQLModel objects are assigned an id when they are stored in the database. It's the row index for the database table.

I would claim that the app does not have to know the ids and they should be hidden from the app. Ids are entirely handled by the sqlmodel backend.

That would also mean that methods like get_entity_by_id or get_all_entities_as_mapping (from id -> entity) should not exist, right?

@vlad-ed-git Please comment.

vlad-ed-git commented 1 year ago

@clstaudt YES instances where IDs are created manually should be deleted (by replacing fake data with real backend implementation). Ids should not be displayed on the front end either (I did remove most of that but the few that may remain are also to be removed) BUT NO to That would also mean that methods like get_entity_by_id or get_all_entities_as_mapping (from id -> entity) should not exist, right? . The screen that displays a list of items, when said item is clicked needs to pass it's id to the details screen. That is why methods like these two are needed. Also the IDs can serve as keys for the items when displayed in a list view (or grid view ) in the mappings otherwise the view will have to come up with another way to identify a specific object which would be redundant.

clstaudt commented 1 year ago

@vlad-ed-git I see, but for example ContactsIntent.get_contact_by_id is never called. Why does the method exist?