codecov[bot]
commented
1 month ago Codecov Report
Attention: Patch coverage is 96.26168% with 4 lines in your changes are missing coverage. Please review.
Project coverage is 69.53%. Comparing base (
20fff67) to head (8d70dc1). Report is 62 commits behind head on develop.
| Files | Patch % | Lines |
|---|---|---|
| perma_web/perma/models.py | 95.12% | 4 Missing :warning: |
Additional details and impacted files
```diff @@ Coverage Diff @@ ## develop #3529 +/- ## ======================================== Coverage 69.53% 69.53% ======================================== Files 48 48 Lines 6801 6785 -16 ======================================== - Hits 4729 4718 -11 + Misses 2072 2067 -5 ```:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Here's another PR with a difficult-to-read diff. I think the individual commits are pretty clear, though, with the exception of the last one!
In preparing this PR, I read through all the application logic for creating new folders, updating folders' metadata, moving them, and deleting them, and tried to use transactions and locks (via
select_for_update) to prevent the application from getting into an inconsistent state. I am hopeful this will prevent any further mangling of our folder trees.I also thoroughly refactored the Folder model's
savemethod, so that it is easier to reason about. Previously, we checked to see if a folder was new, if it had a parent or not, and if its parent had changed in a number of places. With this refactor, each case is handled entirely separately, and shared logic is extracted into reusable helper functions. I also renamed a bunch of stuff, and added comments. Since the diff is hard to read, I think the easiest way to review is to read the new version, uninterrupted, and if desired, compare to the current version.I was able to remove a few redundant calls to the database in the process; I doubt we'll notice much of a change in read ops, but you never know!
I don't have a lot of experience with explicit transactions or with locks; if something looks goofy, it's almost certainly wrong 😄. But, the tests are passing! I think so long as I haven't set us up for competing locks, we're good.
See ENG-854.