ChiliProject is not maintained anymore. Please be advised that there will be no more updates.
We do not recommend that you setup new ChiliProject instances and we urge all existing users to migrate their data to a maintained system, e.g. Redmine. We will provide a migration script later. In the meantime, you can use the instructions by Christian Daehn.
Adding yourself as an issue watcher doesn't update the Watch icons (Bug #432)
Description
- View issue you are not watching
- Add yourself as a watcher using the sidebar form (not the "Watch" icons)
- You are added as a watcher but the "Watch" icons aren't updated
I would expect that the icons are updated along with the sidebar form. #416 added some code that might make this easier.
Related issues
| related to Bug #311: Update the watcher list on "watch"-link click | Closed | 2011-03-31 |
History
Updated by Holger Just at 2011-05-27 06:35 pm
This is basically the other way round from #311.
Updated by Eric Davis at 2011-05-27 09:23 pm
Updated by Felix Schäfer at 2011-05-28 03:09 pm
x_x So WatchersController#destroy (Click on the bin next to a watcher's name) takes "user_id" => "5" as a parameter, whereas WatchersController#new takes "watcher"=>{"user_id"=>"5"}. I'd like to unify that to just "user_id" => "5", any problems with that?
- Assignee set to Felix Schäfer
Updated by Holger Just at 2011-05-28 03:55 pm
Felix Schäfer wrote:
I'd like to unify that to just
"user_id" => "5", any problems with that?
If you do that, we can't mass-create watchers anymore. Although we don't do that currently (apart from issue update), we really should, using some kind of auto-completer like the member add UI in the admin section (just less broken).
Updated by Holger Just at 2011-05-28 03:59 pm
Just re-read it again and noticed that I'm an idiot. So disregard my comment and move along. Nothing to see here...
Updated by Felix Schäfer at 2011-05-28 04:54 pm
The branch is here https://github.com/thegcat/chiliproject/tree/bug/432-update_watch_icons_when_adding_a_watcher_from_the_sidebar , there's no pull request yet as I think the option passing for render_watcher_dropdown could be improved, and the list of "css tags" passed to all these :replace options should probably be set once in the Issue show view and passed to every part needing it as a variable. The functionality is good though, note that I haven't run the tests as I'm out of battery :-/
- Target version set to 2.0.0
- Assignee deleted (
Felix Schäfer) - Status changed from Open to Ready for review
Updated by Felix Schäfer at 2011-05-29 03:11 pm
OK, I've updated the branch linked to in my previous note, I've improved the option passing for the partials, I haven't touched the :replace situation though.
The tests are adapted to the new functionality of the controller, I've thrown one out of the window though, because it was testing if #watch responds to GET requests (which it didn't before because of a verify :method controller), this should be solved by removing the catch-all route though rather than verifying it in the controller.
Updated by Eric Davis at 2011-05-29 06:26 pm
I have code that adds multiple watchers, based on the one used at Admin > Groups > Add Users. I want to make sure we don't change the current code so it can't be applied. Being able to search for and add several watchers at once is a huge UI improvement.
I propose we hold off on this until after 2.0.0-RC1 is out. Then we can add my code and this fix as part of the BugMash.
Updated by Eric Davis at 2011-05-29 06:26 pm
- Target version deleted (
2.0.0)
Updated by Felix Schäfer at 2011-05-30 08:05 am
Eric Davis wrote:
I have code that adds multiple watchers, based on the one used at Admin > Groups > Add Users. I want to make sure we don't change the current code so it can't be applied. Being able to search for and add several watchers at once is a huge UI improvement.
You'll understand that I can't factor in something I haven't seen code for yet, and I don't think we should hold back patches solely because someone has other code "lying around" for it (I agree that a multi-select would be useful, but that's currently not my point). I even seem to remember that was one of the gripes we had in Redmine, I hope we can avoid this situation (or too many of them) here.
Anyway, from an (http) interface point of view, this only changes the creation of a new watcher from POST /watchers/new/WatchersController#new to POST /watchers/WatchersController#create, I haven't changed the passed parameters after all because my proposed solution would have been "wrong" too, and the deletion of watchers has been changed from POST /watchers/destroy/WatchersController#destroy to DELETE /watchers/:id/WatchersController#destroy, the passed parameters have changed accordingly. Both actions now also get passed the replace parameter similar to POST /watchers/watch and POST /watchers/unwatch (which are also wrong and should be refactored to use create and destroy, but that's another story).
Updated by Eric Davis at 2011-05-30 05:36 pm
Felix Schäfer wrote:
You'll understand that I can't factor in something I haven't seen code for yet, and I don't think we should hold back patches solely because someone has other code "lying around" for it (I agree that a multi-select would be useful, but that's currently not my point). I even seem to remember that was one of the gripes we had in Redmine, I hope we can avoid this situation (or too many of them) here.
I understand. I think I found the branch and specific commit. From what it looks like I replaced the params[:user_id] with params[:user_ids] anyways, so I don't think there will be a problem.