TreyWW / MyFinances

MyFinances is a web application that can help you as an individual, or team, manage your finances!
https://docs.myfinances.cloud
GNU Affero General Public License v3.0
85 stars 121 forks source link

Query issue fix #414

Closed Domejko closed 3 weeks ago

Domejko commented 3 weeks ago

Description

Regarding #267

Checklist

What type of PR is this?

Added/updated tests?

Related PRs, Issues etc

TreyWW commented 3 weeks ago

Oh i've just seen what you mean, in our middleware we dont even call the additional fetch for profile. Maybe it's somewhere else. I'll check it out now, i could've swore it was here

TreyWW commented 3 weeks ago

Ah this is what I was thinking of https://github.com/TreyWW/MyFinances/blob/main/backend/models.py#L41

Domejko commented 3 weeks ago

Problem is that both AuthenticationMiddleware and CustomUserMiddleware where using query to fetch the user object. I removed AuthenticationMiddleware from MIDDLEWARE and adjusted CustomUserMiddleware so that it would replace it and populate request.user with our CustomUser instance.

With this changes we don't get double query issue and user profile is also accessible in request.user.

TreyWW commented 3 weeks ago

Amazing. Does this pass all tests still and everything visually work on the site? Just wondering if any Django packages use the middleware or if they will automatically move over to our new one

Domejko commented 3 weeks ago

Yes it was passing tests on my machine and on website everything work as intended. As for AuthenticationMiddleware I did check and it looks like it's not used anywhere else in Django.

I will fix that mypy error and will push that PR.

TreyWW commented 3 weeks ago

Thanks @Domejko, really appreciate the support recently!