Apache Allura™

A great start, this does the job. Some suggestions to make it better and cleaner before we merge it:

There is some duplicated code now in this new code and in the TicketController.delete method. It would be good to refactor that and put the common code in a new method like Ticket.soft_delete ("soft_delete" instead of "delete" would be good, since we're not really deleting it and some models have real delete methods which are different). Make sure to include the M.Shortlink.query.remove line in the common soft_delete method.

A new test to cover this feature be really good. In ForgeTracker/forgetracker/tests/functional/test_root.py there is a test_bulk_edit_notifications method that I think would be good references to create a test for bulk delete. It does a lot of subscribe calls, and assertions about notifications that you don't really need. The important parts of the test to keep are creating the tickets, self.app.post and the ThreadLocalORMSession and M.MonQTask lines (they run background tasks from within the test)

Thanks for this contribution, looking forward to merging it! Let me know if you have any questions about it.

Nice work. One issue, though, when I run the test it fails:

ERROR: forgetracker.tests.functional.test_root.TestFunctionalController.test_bulk_delete
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/brondsem/sf/env-allura/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/Users/brondsem/sf/allura/ForgeTracker/forgetracker/tests/functional/test_root.py", line 2477, in test_bulk_delete
    M.MonQTask.run_ready()
  File "/Users/brondsem/sf/allura/Allura/allura/model/monq_model.py", line 232, in run_ready
    task()
  File "/Users/brondsem/sf/allura/Allura/allura/model/monq_model.py", line 260, in __call__
    self.result = func(*self.args, **self.kwargs)
  File "/Users/brondsem/sf/allura/ForgeTracker/forgetracker/tasks.py", line 47, in bulk_edit
    c.app.globals.update_tickets(**post_data)
  File "/Users/brondsem/sf/allura/ForgeTracker/forgetracker/model/ticket.py", line 393, in update_tickets
    ticket.soft_delete()
  File "/Users/brondsem/sf/allura/ForgeTracker/forgetracker/model/ticket.py", line 1319, in soft_delete
    flash('Ticket successfully deleted')
  File "/Users/brondsem/sf/env-allura/lib/python2.7/site-packages/tg/flash.py", line 18, in __call__
    unicode(message), status, **extra_payload
  File "/Users/brondsem/sf/env-allura/lib/python2.7/site-packages/webflash/__init__.py", line 121, in __call__
    request.environ['webflash.payload'] = payload
  File "/Users/brondsem/sf/env-allura/lib/python2.7/site-packages/paste/registry.py", line 137, in __getattr__
    return getattr(self._current_obj(), attr)
  File "/Users/brondsem/sf/env-allura/lib/python2.7/site-packages/paste/registry.py", line 197, in _current_obj
    'thread' % self.____name__)
TypeError: No object (name: request) has been registered for this thread

I think that flash() call needs to go back in the delete controller method and not in the model method. Flash messages use the current web request, and that isn't available when running as a background task.

Thanks for the feedback. I’ve updated the merge request accordingly and checked the test cases. In the previous case, test cases did not fail in my machine. When executing the test cases, I navigated to ForgeTracker directory and executed ‘nosetests’ command. Could you tell me what was the mistake I’ve done when executing the test cases

Ah.. ok what you did is fine. What happened is that sometimes tests set up globals like request and then that global sticks around unintentionally, and that let the tests pass when you run all of them together. If you run the individual test nosetests forgetracker.tests.functional.test_root:TestFunctionalController.test_bulk_delete or the single file nosetests forgetracker/tests/functional/test_root.py then it fails. Yet another way to run tests is ./run_tests in the top level directory and that will split tests up into chunks and run them in multiple processes (depencing on how many CPUs you have)

Anyway, this is good now, so I'll merge it. Thanks!

Thanks for the clarification and support!