Add consolidation of duplicate LocalSiteProfile objects.

Review Request #12471 — Created July 14, 2022 and submitted

david
Review Board
release-5.0.x
reviewboard

We had two problems that were potentially causing the existence of
duplicate LocalSiteProfile models. The first of these is that this
wasn't using ConcurrencyManager, which handles integrity issues across
separate threads (or servers). The second, more difficult one, is that
some databases (particularly MySQL) were ignoring our unique_together
constraint when one of the relations was NULL. This meant it was
possible to end up with multiple LocalSiteProfile objects with
local_site=None.

This change moves us over to ConcurrencyManager, and implements a
check for the duplicate case when attempting to use
User.get_site_profile. If we do find duplicates, they'll be
consolidated into a single one before returning.

Ran unit tests.

Summary
Add consolidation of duplicate LocalSiteProfile objects.
Description From Last Updated

line too long (80 > 79 characters) Column: 80 Error code: E501

reviewbotreviewbot

line too long (82 > 79 characters) Column: 80 Error code: E501

reviewbotreviewbot

'reviewboard.accounts.models.Profile' imported but unused Column: 1 Error code: F401

reviewbotreviewbot

'reviewboard.site.models.LocalSite' imported but unused Column: 1 Error code: F401

reviewbotreviewbot

Can we give create_if_missing a default?

chipx86chipx86

Most of our code capitalized as "Local Site". Can we have these docs use the same?

chipx86chipx86

Let's make this a numeric list. Makes it much easier to read what's in which slot.

chipx86chipx86

Can we do one per line? We should treat these like a dictionary when we have multiple keyword args. It's …

chipx86chipx86

Same as above, can we do one per line? Minimizes changes if we need to update the query. We may …

chipx86chipx86

We should save all this and permissions using update_fields=, I think?

chipx86chipx86

Can the paren be moved to the next line, and a trailing comma added? We'll probably need to update this …

chipx86chipx86

Swap these.

chipx86chipx86

We should add a step that reloads and verifies resultls.

chipx86chipx86

line too long (82 > 79 characters) Column: 80 Error code: E501

reviewbotreviewbot

line too long (80 > 79 characters) Column: 80 Error code: E501

reviewbotreviewbot
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

david
maubin
  1. Ship It!
  2. 
      
chipx86
  1. 
      
  2. reviewboard/accounts/managers.py (Diff revision 2)
     
     

    Can we give create_if_missing a default?

  3. reviewboard/accounts/managers.py (Diff revision 2)
     
     

    Most of our code capitalized as "Local Site". Can we have these docs use the same?

  4. reviewboard/accounts/managers.py (Diff revision 2)
     
     
     
     

    Let's make this a numeric list. Makes it much easier to read what's in which slot.

  5. reviewboard/accounts/managers.py (Diff revision 2)
     
     
     

    Can we do one per line? We should treat these like a dictionary when we have multiple keyword args. It's much easier to read and to update.

  6. reviewboard/accounts/managers.py (Diff revision 2)
     
     
     

    Same as above, can we do one per line? Minimizes changes if we need to update the query.

    We may also want to use .only(...) with pk, permissions, and the counters. Small optimization, and this is unlikely to be called much, so I'll leave that up to you.

  7. reviewboard/accounts/managers.py (Diff revision 2)
     
     
     
     
     
     
     

    We should save all this and permissions using update_fields=, I think?

  8. reviewboard/accounts/tests/test_local_site_profile.py (Diff revision 2)
     
     
     
     
     
     
     
     

    We should add a step that reloads and verifies resultls.

  9. 
      
david
Review request changed

Commits:

Summary
-
Add consolidation of duplicate LocalSiteProfile objects.
+
Add consolidation of duplicate LocalSiteProfile objects.

Diff:

Revision 3 (+344 -26)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

david
chipx86
  1. 
      
  2. reviewboard/accounts/managers.py (Diff revisions 2 - 4)
     
     

    Can the paren be moved to the next line, and a trailing comma added? We'll probably need to update this list at some point.

    It'd be nice to alphabetize it, too.

  3. 
      
david
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-5.0.x (906cbbc)
Loading...