widgets of Admin Dashboard do not reflect changes

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

yuri_mingo
Review Board
master
2753
reviewboard
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 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

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)
     
     
    Col: 1
     W191 indentation contains tabs
    
  3. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  4. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Col: 80
     E501 line too long (104 > 79 characters)
    
  5. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  6. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Col: 1
     W191 indentation contains tabs
    
  7. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Col: 65
     E242 tab after ','
    
  8. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Col: 1
     W191 indentation contains tabs
    
  9. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  10. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Col: 1
     E302 expected 2 blank lines, found 1
    
  11. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Col: 18
     E231 missing whitespace after ','
    
  12. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Col: 1
     E302 expected 2 blank lines, found 1
    
  13. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Col: 1
     W191 indentation contains tabs
    
  14. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  15. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Col: 80
     E501 line too long (104 > 79 characters)
    
  16. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Col: 1
     W191 indentation contains tabs
    
  17. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  18. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Col: 65
     E242 tab after ','
    
  19. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Col: 1
     W191 indentation contains tabs
    
  20. reviewboard/admin/widgets.py (Diff revision 1)
     
     
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  21. reviewboard/diffviewer/parser.py (Diff revision 1)
     
     
    Col: 23
     E221 multiple spaces before operator
    
  22. Col: 80
     E501 line too long (93 > 79 characters)
    
  23. Col: 80
     E501 line too long (84 > 79 characters)
    
  24. reviewboard/reviews/views.py (Diff revision 1)
     
     
    Col: 9
     E128 continuation line under-indented for visual indent
    
  25. reviewboard/scmtools/perforce.py (Diff revision 1)
     
     
    Col: 41
     E126 continuation line over-indented for hanging indent
    
  26. reviewboard/scmtools/perforce.py (Diff revision 1)
     
     
    Col: 41
     E126 continuation line over-indented for hanging indent
    
  27. reviewboard/scmtools/perforce.py (Diff revision 1)
     
     
    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)
     
     
    Col: 1
     W191 indentation contains tabs
    
  3. reviewboard/admin/widgets.py (Diff revision 2)
     
     
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  4. reviewboard/admin/widgets.py (Diff revision 2)
     
     
    Col: 80
     E501 line too long (104 > 79 characters)
    
  5. reviewboard/admin/widgets.py (Diff revision 2)
     
     
    Col: 1
     W191 indentation contains tabs
    
  6. reviewboard/admin/widgets.py (Diff revision 2)
     
     
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  7. reviewboard/admin/widgets.py (Diff revision 2)
     
     
    Col: 65
     E242 tab after ','
    
  8. reviewboard/admin/widgets.py (Diff revision 2)
     
     
    Col: 1
     W191 indentation contains tabs
    
  9. reviewboard/admin/widgets.py (Diff revision 2)
     
     
    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)
     
     
     
    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)
     
     
    Col: 1
     W191 indentation contains tabs
    
  3. reviewboard/admin/widgets.py (Diff revision 3)
     
     
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  4. reviewboard/admin/widgets.py (Diff revision 3)
     
     
    Col: 80
     E501 line too long (104 > 79 characters)
    
  5. reviewboard/admin/widgets.py (Diff revision 3)
     
     
    Col: 1
     W191 indentation contains tabs
    
  6. reviewboard/admin/widgets.py (Diff revision 3)
     
     
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  7. reviewboard/admin/widgets.py (Diff revision 3)
     
     
    Col: 65
     E242 tab after ','
    
  8. reviewboard/admin/widgets.py (Diff revision 3)
     
     
    Col: 1
     W191 indentation contains tabs
    
  9. reviewboard/admin/widgets.py (Diff revision 3)
     
     
    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)
     
     
     
    Separate by one blank line.
  3. reviewboard/admin/__init__.py (Diff revision 3)
     
     
     
     
    Two blank lines here.
  4. reviewboard/admin/signals.py (Diff revision 3)
     
     
     
     
     
    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)
     
     
    You have an extra blank line here.
  6. reviewboard/admin/widgets.py (Diff revision 3)
     
     
     
    Imports must be listed in alphabetical order.
  7. reviewboard/admin/widgets.py (Diff revision 3)
     
     
     
     
     
     
     
     
     
     
     
    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)
     
     
    The key should be defined only once, since it's common between the functions.
  9. reviewboard/admin/widgets.py (Diff revision 3)
     
     
    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)
     
     
     
    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)
     
     
    Col: 1
     E302 expected 2 blank lines, found 1
    
  3. reviewboard/admin/widgets.py (Diff revision 4)
     
     
    Col: 1
     W293 blank line contains whitespace
    
  4. reviewboard/admin/widgets.py (Diff revision 4)
     
     
    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: Closed (submitted)

Change Summary:

Pushed to release-1.7.x (931b172). Thanks!
Loading...