widgets of Admin Dashboard do not reflect changes

Review Request #3780 — Created Jan. 19, 2013 and submitted

Information

Review Board
master

Reviewers

widgets of Admin Dashboard do not reflect changes

Fixed Issue 2753 (Some Admin Dashboard widgets do not reflect changes to the data they contain).
Admin Dashboard widgets did not reflect when we add the repository because Admin Dashboard could not detect changing in this cache form.
So I change form of cache, I add username and syncnum which is value for invalid old caches.

[reviewboard/admin/__init__.py]
	* bring post_save.connect and post_delete from signals.py.
[reviewboard/admin/widgets.py]
	* add username and syncnum, is value for invalid old caches, to generate_cache_key().
[reviewboard/admin/signals.py]
	* change _delete_widget_cache() to increment syncnum.
	* remove post_save.connect and post_delete.
I check the Admin Dashboard after add a repository, then I can see correct data on Repositories widget.
Description From Last Updated

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

Col: 80 E501 line too long (104 > 79 characters)

reviewbotreviewbot

Col: 1 E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 65 E242 tab after ','

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

Col: 1 E302 expected 2 blank lines, found 1

reviewbotreviewbot

Col: 18 E231 missing whitespace after ','

reviewbotreviewbot

Col: 1 E302 expected 2 blank lines, found 1

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

Col: 80 E501 line too long (104 > 79 characters)

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

Col: 65 E242 tab after ','

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

Col: 23 E221 multiple spaces before operator

reviewbotreviewbot

Col: 80 E501 line too long (93 > 79 characters)

reviewbotreviewbot

Col: 80 E501 line too long (84 > 79 characters)

reviewbotreviewbot

Col: 9 E128 continuation line under-indented for visual indent

reviewbotreviewbot

Col: 41 E126 continuation line over-indented for hanging indent

reviewbotreviewbot

Col: 41 E126 continuation line over-indented for hanging indent

reviewbotreviewbot

Col: 80 E501 line too long (82 > 79 characters)

reviewbotreviewbot

I think these will get triggered whenever *any* model deletes or saves, which is a lot more than we want …

daviddavid

Col: 1 E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 80 E501 line too long (104 > 79 characters)

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

Col: 65 E242 tab after ','

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

Separate by one blank line.

chipx86chipx86

Two blank lines here.

chipx86chipx86

Imports go into three groups: 1) Python-provided modules 2) Third-party modules (django, djblets) 3) Modules as part of the current …

chipx86chipx86

You have an extra blank line here.

chipx86chipx86

Imports must be listed in alphabetical order.

chipx86chipx86

This will cause problems when you have multiple Review Board sites sharing the same cache. You should be using djblets.util.misc.make_cache_key …

chipx86chipx86

The key should be defined only once, since it's common between the functions.

chipx86chipx86

I don't understand this logic. Every time we get the current number, we increment it? Either the function is named …

chipx86chipx86

This doesn't look correct. You're adding a new entry and then incrementing, which isn't what we want. Look at djblets/siteconfig/models.py:save, …

chipx86chipx86

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

Col: 80 E501 line too long (104 > 79 characters)

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

Col: 65 E242 tab after ','

reviewbotreviewbot

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

Col: 1 E302 expected 2 blank lines, found 1

reviewbotreviewbot

Col: 1 W293 blank line contains whitespace

reviewbotreviewbot

Col: 23 E711 comparison to None should be 'if cond is not None

reviewbotreviewbot
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        reviewboard/admin/checks.py
        reviewboard/diffviewer/parser.py
        reviewboard/settings.py
        reviewboard/attachments/models.py
        reviewboard/__init__.py
        reviewboard/admin/widgets.py
        reviewboard/scmtools/perforce.py
        reviewboard/reviews/models.py
        reviewboard/scmtools/errors.py
        reviewboard/scmtools/core.py
        reviewboard/scmtools/forms.py
        reviewboard/cmdline/rbsite.py
        reviewboard/scmtools/tests.py
        reviewboard/admin/signals.py
        reviewboard/scmtools/certs.py
        reviewboard/admin/__init__.py
        reviewboard/reviews/views.py
        reviewboard/reviews/management/commands/index.py
        setup.py
        reviewboard/scmtools/svn.py
      Ignored Files:
        reviewboard/cmdline/conf/apache-fastcgi.conf.in
        reviewboard/static/rb/css/reviews.less
        reviewboard/templates/diffviewer/changeindex_entry.html
        docs/releasenotes/reviewboard/1.7.2.txt
        reviewboard/templates/admin/repository_fields.js
        reviewboard/templates/reviews/comments_dlg.html
        reviewboard/static/rb/js/diffviewer.js
        reviewboard/static/rb/js/views/commentDialogView.js
        reviewboard/templates/admin/widgets/w-stats-large.html
        reviewboard/static/rb/js/reviews.js
        reviewboard/static/rb/js/views/imageReviewableView.js
        docs/releasenotes/reviewboard/index.txt
        reviewboard/templates/js/tests.html
        reviewboard/templates/admin/repository_confirmations.html
        reviewboard/static/rb/js/models/commentEditorModel.js
        reviewboard/cmdline/conf/apache-wsgi.conf.in
        reviewboard/static/rb/js/views/abstractCommentBlockView.js
        reviewboard/static/rb/js/views/tests/commentDialogViewTests.js
        reviewboard/templates/base.html
        reviewboard/static/rb/js/datastore.js
        reviewboard/static/rb/js/views/abstractReviewableView.js
        AUTHORS
        reviewboard/static/rb/js/views/regionCommentBlockView.js
        reviewboard/cmdline/conf/apache-modpython.conf.in
        reviewboard/static/rb/js/models/tests/commentEditorModelTests.js
    
    
  2. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     W191 indentation contains tabs
    
  3. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  4. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Show all issues
    Col: 80
     E501 line too long (104 > 79 characters)
    
  5. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  6. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     W191 indentation contains tabs
    
  7. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Show all issues
    Col: 65
     E242 tab after ','
    
  8. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     W191 indentation contains tabs
    
  9. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  10. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     E302 expected 2 blank lines, found 1
    
  11. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Show all issues
    Col: 18
     E231 missing whitespace after ','
    
  12. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     E302 expected 2 blank lines, found 1
    
  13. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     W191 indentation contains tabs
    
  14. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  15. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Show all issues
    Col: 80
     E501 line too long (104 > 79 characters)
    
  16. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     W191 indentation contains tabs
    
  17. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  18. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Show all issues
    Col: 65
     E242 tab after ','
    
  19. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     W191 indentation contains tabs
    
  20. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Show all issues
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  21. reviewboard/diffviewer/parser.py (Diff revision 1)
     
     
    Show all issues
    Col: 23
     E221 multiple spaces before operator
    
  22. Show all issues
    Col: 80
     E501 line too long (93 > 79 characters)
    
  23. Show all issues
    Col: 80
     E501 line too long (84 > 79 characters)
    
  24. reviewboard/reviews/views.py (Diff revision 1)
     
     
    Show all issues
    Col: 9
     E128 continuation line under-indented for visual indent
    
  25. reviewboard/scmtools/perforce.py (Diff revision 1)
     
     
    Show all issues
    Col: 41
     E126 continuation line over-indented for hanging indent
    
  26. reviewboard/scmtools/perforce.py (Diff revision 1)
     
     
    Show all issues
    Col: 41
     E126 continuation line over-indented for hanging indent
    
  27. reviewboard/scmtools/perforce.py (Diff revision 1)
     
     
    Show all issues
    Col: 80
     E501 line too long (82 > 79 characters)
    
  28. 
      
chipx86
  1. It looks like this change has parts of other changes in it. Make sure master is up-to-date with origin/master, and that your branch is rebased onto master.
    
    Also, the summary and description of your change should be self-explanatory, covering what this does and why. If you look at other review requests by myself in the All Review Requests on this server, you should see some examples.
    1. Also, you'll need to cover all the testing you did in Testing Done.
  2. 
      
YU
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        reviewboard/admin/widgets.py
        reviewboard/admin/signals.py
        reviewboard/admin/__init__.py
      Ignored Files:
    
    
  2. reviewboard/admin/widgets.py (Diff revision 2)
     
     
    Show all issues
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  3. reviewboard/admin/widgets.py (Diff revision 2)
     
     
    Show all issues
    Col: 1
     W191 indentation contains tabs
    
  4. reviewboard/admin/widgets.py (Diff revision 2)
     
     
    Show all issues
    Col: 80
     E501 line too long (104 > 79 characters)
    
  5. reviewboard/admin/widgets.py (Diff revision 2)
     
     
    Show all issues
    Col: 1
     W191 indentation contains tabs
    
  6. reviewboard/admin/widgets.py (Diff revision 2)
     
     
    Show all issues
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  7. reviewboard/admin/widgets.py (Diff revision 2)
     
     
    Show all issues
    Col: 65
     E242 tab after ','
    
  8. reviewboard/admin/widgets.py (Diff revision 2)
     
     
    Show all issues
    Col: 1
     W191 indentation contains tabs
    
  9. reviewboard/admin/widgets.py (Diff revision 2)
     
     
    Show all issues
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  10. 
      
YU
david
  1. 
      
    1. In addition to this code change, I'd like you to do some clean-up on the review request itself:
      - Add the bug number to the "Bugs" field.
      - Go through the reviewbot reviews and click "Fixed" on all the open issues which you've already addressed.
  2. reviewboard/admin/__init__.py (Diff revision 2)
     
     
     
    Show all issues
    I think these will get triggered whenever *any* model deletes or saves, which is a lot more than we want (so much so that it eliminates the utility of the cache). Probably what we want to add is this:
    
    from reviewboard.reviews.models import Group
    from reviewboard.scmtools.models import Repository
    
    post_save.connect(_delete_widget_cache, sender=Group)
    post_save.connect(_delete_widget_cache, sender=Repository)
    post_delete.connect(_delete_widget_cache, sender=Group)
    post_delete.connect(_delete_widget_cache, sender=Repository)
    
    and do similarly for each model which is represented in the dashboard and has this issue.
  3. 
      
YU
YU
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        reviewboard/admin/__init__.py
        reviewboard/admin/signals.py
        reviewboard/admin/widgets.py
      Ignored Files:
    
    
  2. reviewboard/admin/widgets.py (Diff revision 3)
     
     
    Show all issues
    Col: 1
     W191 indentation contains tabs
    
  3. reviewboard/admin/widgets.py (Diff revision 3)
     
     
    Show all issues
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  4. reviewboard/admin/widgets.py (Diff revision 3)
     
     
    Show all issues
    Col: 80
     E501 line too long (104 > 79 characters)
    
  5. reviewboard/admin/widgets.py (Diff revision 3)
     
     
    Show all issues
    Col: 1
     W191 indentation contains tabs
    
  6. reviewboard/admin/widgets.py (Diff revision 3)
     
     
    Show all issues
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  7. reviewboard/admin/widgets.py (Diff revision 3)
     
     
    Show all issues
    Col: 65
     E242 tab after ','
    
  8. reviewboard/admin/widgets.py (Diff revision 3)
     
     
    Show all issues
    Col: 1
     W191 indentation contains tabs
    
  9. reviewboard/admin/widgets.py (Diff revision 3)
     
     
    Show all issues
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  10. 
      
chipx86
  1. I want to see more testing done. I need to know that the following things work:
    
    1) The dev server runs correctly.
    
    2) Unit tests all run.
    
    This should also include some new unit tests to make sure that the cache resets when those models are updated (each combination needs a test). These tests would go in admin/tests.py.
  2. reviewboard/admin/__init__.py (Diff revision 3)
     
     
     
    Show all issues
    Separate by one blank line.
  3. reviewboard/admin/__init__.py (Diff revision 3)
     
     
     
     
    Show all issues
    Two blank lines here.
  4. reviewboard/admin/signals.py (Diff revision 3)
     
     
     
     
     
    Show all issues
    Imports go into three groups:
    
    1) Python-provided modules
    2) Third-party modules (django, djblets)
    3) Modules as part of the current project
    
    There should be a blank line between here.
    
    So in this case, the reviewboard import must go after the other imports, with a blank line separating them.
  5. reviewboard/admin/signals.py (Diff revision 3)
     
     
    Show all issues
    You have an extra blank line here.
  6. reviewboard/admin/widgets.py (Diff revision 3)
     
     
     
    Show all issues
    Imports must be listed in alphabetical order.
  7. reviewboard/admin/widgets.py (Diff revision 3)
     
     
     
     
     
     
     
     
     
     
     
    Show all issues
    This will cause problems when you have multiple Review Board sites sharing the same cache.
    
    You should be using djblets.util.misc.make_cache_key for generating the keys.
    
    Also, each function should have a docstring. This is always in the format of:
    
    
    """One-line summary."""
    
    or:
    
    """One-line summary
    
    Multi-line description.
    """
  8. reviewboard/admin/widgets.py (Diff revision 3)
     
     
    Show all issues
    The key should be defined only once, since it's common between the functions.
  9. reviewboard/admin/widgets.py (Diff revision 3)
     
     
    Show all issues
    I don't understand this logic. Every time we get the current number, we increment it? Either the function is named oddly, or the behavior is wrong.
  10. reviewboard/admin/widgets.py (Diff revision 3)
     
     
     
    Show all issues
    This doesn't look correct. You're adding a new entry and then incrementing, which isn't what we want.
    
    Look at djblets/siteconfig/models.py:save, and see how it's handling setting the last sync number.
    
    In general, that's the logic you should be basing things off of.
  11. 
      
YU
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        reviewboard/admin/__init__.py
        reviewboard/admin/signals.py
        reviewboard/admin/widgets.py
      Ignored Files:
        #TAGS#
        reviewboard/tags
        TAGS
    
    
  2. reviewboard/admin/widgets.py (Diff revision 4)
     
     
    Show all issues
    Col: 1
     E302 expected 2 blank lines, found 1
    
  3. reviewboard/admin/widgets.py (Diff revision 4)
     
     
    Show all issues
    Col: 1
     W293 blank line contains whitespace
    
  4. reviewboard/admin/widgets.py (Diff revision 4)
     
     
    Show all issues
    Col: 23
     E711 comparison to None should be 'if cond is not None
  5. 
      
YU
YU
YU
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        reviewboard/admin/__init__.py
        reviewboard/admin/signals.py
        reviewboard/admin/widgets.py
      Ignored Files:
    
    
  2. 
      
david
  1. Ship It!
  2. 
      
YU
Review request changed
Status:
Completed
Change Summary:
Pushed to release-1.7.x (931b172). Thanks!