refactor: reduce DB updates by traits update requests without real changes in the data

[devapro]

Thanks for submitting a PR! Please check the boxes below:

• [ ] I have added information to docs/ if required so people know about the feature!
• [x] I have filled in the "Changes" section below?
• [ ] I have filled in the "How did you test this code" section below?
• [x] I have used a Conventional Commit title for this Pull Request

Changes

It is common case for mobile app to set traits on the app start, because some properties can be changed offline or by request from another device with the same account etc. However in this case db is update even if no changes in the data. This change should reduce updates by getting list of traits from db than compares the existing and incoming data, and only add/update when the data actually changes. This strategy drastically reduces unnecessary UPDATE operations in case when the clients send requests with the same data.

How did you test this code?

manually

Please describe.

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment

Name	Status	Preview	Comments	Updated (UTC)
docs	⬜️ Ignored (Inspect)	Visit Preview		Aug 9, 2024 1:48pm

[vercel[bot]]

@devapro is attempting to deploy a commit to the Flagsmith Team on Vercel.

A member of the Team first needs to authorize it.

[sentry[bot]]

🔍 Existing Issues For Review

Your pull request is modifying functions with the following pre-existing issues:

📄 File: api/environments/identities/traits/views.py

Function	Unhandled Issue
bulk_create	KeyError: 'identifier' /api/v1/traits/bulk/ Event Count: 1

---

_{Did you find this useful? React with a 👍 or 👎}

[github-actions[bot]]

Uffizzi Ephemeral Environment deployment-56255

:cloud: https://app.uffizzi.com/github.com/Flagsmith/flagsmith/pull/4451

:page_facing_up: View Application Logs etc.

What is Uffizzi? Learn more!

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 97.18%. Comparing base (5cbdd7f) to head (39d82cd). Report is 121 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4451      +/-   ##
==========================================
+ Coverage   96.85%   97.18%   +0.32%     
==========================================
  Files        1170     1163       -7     
  Lines       38812    40289    +1477     
==========================================
+ Hits        37592    39153    +1561     
+ Misses       1220     1136      -84     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[matthewelwell]

@devapro just checking in to see if you had time to look at my recent review comments and whether you might have time to update the PR?

[devapro]

Sorry for long response, will check your comments next week.

[devapro]

Thanks for the changes @devapro . I've added a couple more comments.

As per my previous review though, it's still missing any sort of testing coverage.

I checked existing unit tests and looks like it is covered all logic that I refactored. I think will be good to add test that checks case when db is not called, but my knowledge in python very limited, I didn't find an example of how to do it in current code.

[matthewelwell]

I think will be good to add test that checks case when db is not called, but my knowledge in python very limited, I didn't find an example of how to do it in current code.

You can just copy one of the existing tests, manipulate the data to be what you want and then use the django_assert_num_queries fixture to do something like:

with django_assert_num_queries(0):
    ... 

You'll find plenty of usages of this fixture throughout the code base.

[devapro]

I think will be good to add test that checks case when db is not called, but my knowledge in python very limited, I didn't find an example of how to do it in current code.

You can just copy one of the existing tests, manipulate the data to be what you want and then use the django_assert_num_queries fixture to do something like:

with django_assert_num_queries(0):
    ... 

You'll find plenty of usages of this fixture throughout the code base.

I can't find existing test for method that I changed. It is not simple for me to understand how to write new test for specific method.

Didn't manage to run tests locally, for running project I am using docker + mounting specific files with changes, because can't install all required dependencies.