tenant.propertyID

Technically, tenant's don't always have a property id. If this is a brand new lease then the tenant would not have a relationship with the property. Is there a reason why this is being changed?

Perhaps it would've been more accurate for the existing test to do create_tenant(propertyID=None)
or
We can create a new fixture called create_unhoused_tenant()

I'm actually not a fan of having propertyID on the tenant model, but that's an issue for another day.

The problem here stems from the fact that firing off both of these functions leads to two properties with the same name being added. This happens because create_tenant() creates a property, and create_property() creates a property with the same name!

Like you said, maybe creating a tenant shouldn't create a property? I've changed things a bit so that we can do create_tenant(propertyID=None), but the request is now failing with code 400. I'm unsure what valid_payload() should do when it gets a None value for property id -- is that a problem? I'm totally unsure of what that request ends up looking like.

Also, I'm still understanding the database model here, but shouldn't all leases involve a tenant id and a property id? How could we add a lease without a property id? Just from the name, I assumed that leases were what bound tenants to properties.

ah I understand the reason for these changes now. Because in the original tests they were creating two properties. I should've read your commit comments! Sorry I'm not used to looking, because I don't usually see that. Thanks for adding those. Next time I will try to be better.

The problem here stems from the fact that firing off both of these functions leads to two properties with the same name being added

This is easily fixed. I'm actually going to be creating issues to implement faker on all of the factories. This will allow factories to be created with dynamic data. You can see an implementation of this in the User factory in #154 specifically on the email field.

Like you said, maybe creating a tenant shouldn't create a property? I've changed things a bit so that we can do create_tenant(propertyID=None), but the request is now failing with code 400. I'm unsure what valid_payload() should do when it gets a None value for property id -- is that a problem? I'm totally unsure of what that request ends up looking like.

Yeah, The factory is not setup to accept None. That could be tweaked. It could look something like this:

def create_tenant():
    def _create_tenant(property=None):
        tenant = TenantModel(
                firstName="firstName",
                lastName="lastName",
                phone="phone",
                propertyID=property.id if property else None,
                staffIDs=[],
                unitNum=3
            )
        tenant.save_to_db()
        return tenant
    yield _create_tenant

And then (unfortunately) the lease factory and then some of the tests would need to be updated. I would opt for creating an unhoused tenant factory and use the faker library in both the new and existing factory. The 400 is being returned because there is probably a validation error, hence the valid_payload being sent is not valid.

The request being sent is the valid payload. so it would like something like this:

           {
                'dateTimeStart': '11/25/2020 17:14:01',
                'dateTimeEnd': '11/25/2021 17:14:20',
                'tenantID': 1,
                'propertyID': 1
            }

Also, I'm still understanding the database model here, but shouldn't all leases involve a tenant id and a property id? How could we add a lease without a property id? Just from the name, I assumed that leases were what bound tenants to properties.

Yes you are absolutely correct here. That is why I'm NOT a fan of having a propertyID on the tenant model, because that creates a situation where we need to always update two columns in two separate tables. One propertyID on the lease, and one propertyID on the tenant, and they should always be the same. Leaving it this way could eventually cause our database to have erroneous data. So yes. You are absolutely correct and I agree with you here. The lease should bound the tenant to the property. If you would like to create an issue for this that would be outstanding.

I'm also fine with removing the propertyID on the tenant model in this PR too. If that is something you would like to do.

Hm. Strangely, stuff started working after I just rebased. Tenant model is still exactly the same. Let's get this merged and do the other stuff in other PRs. To be clear, the other change we're talking about is removing propertyID on tenant model? Is there also a change to the lease model?

FYI: Created issue codeforpdx/dwellingly-app/issues/455 to address the double relationship issue. Thanks for bringing it up again.

To be clear, the other change we're talking about is removing propertyID on tenant model? Is there also a change to the lease model?

I think there was something else. I will need to jog my memory and I will get back to you. Thanks for getting this rebased. I will get it merged soon. Thank you

@htharker42 recently updated the lease model. So I think we're all set there.

Issue codeforpdx/dwellingly-app/issues/459 created to get Faker in use on all the factories. I was going to create an issue to update these tests afterwards, but I think these tests will actually end up being deleted in the future, because the lease endpoint is not being utilized by the FE UI.