widgets of Admin Dashboard do not reflect changes
Review Request #3780 — Created Jan. 19, 2013 and submitted
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 |
 reviewbot
|
|
|
Col: 1 E101 indentation contains mixed spaces and tabs |
reviewbot
|
|
|
Col: 80 E501 line too long (104 > 79 characters) |
reviewbot
|
|
|
Col: 1 E101 indentation contains mixed spaces and tabs |
reviewbot
|
|
|
Col: 1 W191 indentation contains tabs |
reviewbot
|
|
|
Col: 65 E242 tab after ',' |
reviewbot
|
|
|
Col: 1 W191 indentation contains tabs |
reviewbot
|
|
|
Col: 1 E101 indentation contains mixed spaces and tabs |
reviewbot
|
|
|
Col: 1 E302 expected 2 blank lines, found 1 |
reviewbot
|
|
|
Col: 18 E231 missing whitespace after ',' |
reviewbot
|
|
|
Col: 1 E302 expected 2 blank lines, found 1 |
reviewbot
|
|
|
Col: 1 W191 indentation contains tabs |
reviewbot
|
|
|
Col: 1 E101 indentation contains mixed spaces and tabs |
reviewbot
|
|
|
Col: 80 E501 line too long (104 > 79 characters) |
reviewbot
|
|
|
Col: 1 W191 indentation contains tabs |
reviewbot
|
|
|
Col: 1 E101 indentation contains mixed spaces and tabs |
reviewbot
|
|
|
Col: 65 E242 tab after ',' |
reviewbot
|
|
|
Col: 1 W191 indentation contains tabs |
reviewbot
|
|
|
Col: 1 E101 indentation contains mixed spaces and tabs |
reviewbot
|
|
|
Col: 23 E221 multiple spaces before operator |
reviewbot
|
|
|
Col: 80 E501 line too long (93 > 79 characters) |
reviewbot
|
|
|
Col: 80 E501 line too long (84 > 79 characters) |
reviewbot
|
|
|
Col: 9 E128 continuation line under-indented for visual indent |
reviewbot
|
|
|
Col: 41 E126 continuation line over-indented for hanging indent |
reviewbot
|
|
|
Col: 41 E126 continuation line over-indented for hanging indent |
reviewbot
|
|
|
Col: 80 E501 line too long (82 > 79 characters) |
reviewbot
|
|
|
I think these will get triggered whenever *any* model deletes or saves, which is a lot more than we want … |
 david
|
|
|
Col: 1 E101 indentation contains mixed spaces and tabs |
reviewbot
|
|
|
Col: 1 W191 indentation contains tabs |
reviewbot
|
|
|
Col: 80 E501 line too long (104 > 79 characters) |
reviewbot
|
|
|
Col: 1 W191 indentation contains tabs |
reviewbot
|
|
|
Col: 1 E101 indentation contains mixed spaces and tabs |
reviewbot
|
|
|
Col: 65 E242 tab after ',' |
reviewbot
|
|
|
Col: 1 W191 indentation contains tabs |
reviewbot
|
|
|
Col: 1 E101 indentation contains mixed spaces and tabs |
reviewbot
|
|
|
Separate by one blank line. |
 chipx86
|
|
|
Two blank lines here. |
chipx86
|
|
|
Imports go into three groups: 1) Python-provided modules 2) Third-party modules (django, djblets) 3) Modules as part of the current … |
chipx86
|
|
|
You have an extra blank line here. |
chipx86
|
|
|
Imports must be listed in alphabetical order. |
chipx86
|
|
|
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 … |
chipx86
|
|
|
The key should be defined only once, since it's common between the functions. |
chipx86
|
|
|
I don't understand this logic. Every time we get the current number, we increment it? Either the function is named … |
chipx86
|
|
|
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, … |
chipx86
|
|
|
Col: 1 W191 indentation contains tabs |
reviewbot
|
|
|
Col: 1 E101 indentation contains mixed spaces and tabs |
reviewbot
|
|
|
Col: 80 E501 line too long (104 > 79 characters) |
reviewbot
|
|
|
Col: 1 W191 indentation contains tabs |
reviewbot
|
|
|
Col: 1 E101 indentation contains mixed spaces and tabs |
reviewbot
|
|
|
Col: 65 E242 tab after ',' |
reviewbot
|
|
|
Col: 1 W191 indentation contains tabs |
reviewbot
|
|
|
Col: 1 E101 indentation contains mixed spaces and tabs |
reviewbot
|
|
|
Col: 1 E302 expected 2 blank lines, found 1 |
reviewbot
|
|
|
Col: 1 W293 blank line contains whitespace |
reviewbot
|
|
|
Col: 23 E711 comparison to None should be 'if cond is not None |
reviewbot
|
-
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.
-
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: -
-
-
-
-
-
-
-
YU
- Change Summary:
-
fix description and testing done.
- Summary:
-
fix the widgets' cachewidgets of Admin Dashboard do not reflect changes
- Description:
-
~ fix the widgets' cache
~ 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. - - Release Review Board 1.7.2.
- Testing Done:
-
+ I check the Admin Dashboard after add a repository, then I can see correct data on Repositories widget.
-
-
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.
YU
- Bugs:
YU
- Change Summary:
-
change to delete specified model's cache, and fix a bug (in increment_sync_num [widgets.py])
-
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: -
-
-
-
-
-
-
-
-
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.
-
-
-
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.
-
-
-
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. """
-
-
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.
-
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.
-
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 -
-
-
YU
- Change Summary:
-
1) The dev server runs correctly. I make sure the dev_server runs correctly when I add and remove repository. 2) Unit tests all run. Unit tests has no problems.
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