Add to GUI the ability to delete charms & bundles.

Add to GUI the ability to delete charms & bundles.

On the charm detail page, a 'Delete' link is added if the user has edit
permission.

On the bundle detail page, a new 'Actions' section is added that has 'Feature'
and 'Delete' links.

We talked about removing the charmworld/scripts/remove_charm.py file but that
isn't done in this branch.

QA: ingest some charms and bundles.  While not logged in, visit the charm and
bundle pages and notice you don't see the 'Delete' links.

Log in.

Now see the links.  Click 'Delete' then cancel or submit without checking the
box.  Note the charm isn't deleted.  Now delete it for real.  Note that it is
gone.  

Repeat for bundles.

Try featuring a bundle.

Late breaking thought:  deleting a charm that a bundle depends upon may cause
it to be undeployable.  Refuse to delete a charm if a bundle depends on it or
just leave it be?  Note this is no worse than the remove_charm.py script,
though it is easier to do.

https://code.launchpad.net/~bac/charmworld/remove-bundles/+merge/211951

(do not edit description out of merge proposal)

[bac, 2014-03-20 14:25:03]

Please take a look.

[bac, 2014-03-20 14:36:58]

Notes for reviewer

https://codereview.appspot.com/78220043/diff/1/charmworld/jobs/ingest.py
File charmworld/jobs/ingest.py (right):

https://codereview.appspot.com/78220043/diff/1/charmworld/jobs/ingest.py#newc...
charmworld/jobs/ingest.py:725: log.info('Invalid revision %s', rev_raw)
Drive-by fix to an error that is seen on ingest.
The log.info should be part of the exception handling.  Will be fixed in next
patch set.

https://codereview.appspot.com/78220043/diff/1/charmworld/jobs/tests/test_wor...
File charmworld/jobs/tests/test_worker.py (right):

https://codereview.appspot.com/78220043/diff/1/charmworld/jobs/tests/test_wor...
charmworld/jobs/tests/test_worker.py:116: worker = QueueWorker(self.db, None,
ingest_jobs)
Change jobs to be a tuple not a dictionary to match the new interface.  Drive-by
name changes for readability.

https://codereview.appspot.com/78220043/diff/1/charmworld/jobs/worker.py
File charmworld/jobs/worker.py (right):

https://codereview.appspot.com/78220043/diff/1/charmworld/jobs/worker.py#newc...
charmworld/jobs/worker.py:65: for queue, jobs in self.ingest_jobs:
ingest_jobs is now a tuple not a dictionary.  This change is made to ensure the
order of processing is charms then bundles.  Using .values() from a dict did not
ensure the order.

https://codereview.appspot.com/78220043/diff/1/charmworld/jobs/worker.py#newc...
charmworld/jobs/worker.py:121: )
The change to not use a dict.

https://codereview.appspot.com/78220043/diff/1/charmworld/scripts/remove_char...
File charmworld/scripts/remove_charm.py (right):

https://codereview.appspot.com/78220043/diff/1/charmworld/scripts/remove_char...
charmworld/scripts/remove_charm.py:51: delete_charm(charm_data, db,
index_client)
Refactored charm deletion so it could be re-used.

https://codereview.appspot.com/78220043/diff/1/charmworld/search.py
File charmworld/search.py (right):

https://codereview.appspot.com/78220043/diff/1/charmworld/search.py#newcode537
charmworld/search.py:537: for doc in result['docs']]
In some situations '_source' was not available.  Be defensive.

[bac, 2014-03-20 14:51:11]

Please take a look.

[benji, 2014-03-20 16:14:17]

LGTM

https://codereview.appspot.com/78220043/diff/1/charmworld/jobs/worker.py
File charmworld/jobs/worker.py (right):

https://codereview.appspot.com/78220043/diff/1/charmworld/jobs/worker.py#newc...
charmworld/jobs/worker.py:56: for _, jobs in self.ingest_jobs:
I might like your construct better, but it occurs to me that this idiom work
too, FYI:

  for jobs in zip(*self.ingest_jobs)[1]:
    ...

https://codereview.appspot.com/78220043/diff/1/charmworld/jobs/worker.py#newc...
charmworld/jobs/worker.py:121: )
On 2014/03/20 14:36:59, bac wrote:
> The change to not use a dict.

I like it how only the syntax changed while all the code stayed put.  This might
be a design goal for a good language.

https://codereview.appspot.com/78220043/diff/20001/charmworld/tests/test_mode...
File charmworld/tests/test_models.py (right):

https://codereview.appspot.com/78220043/diff/20001/charmworld/tests/test_mode...
charmworld/tests/test_models.py:2374: # A convenience constructor is provided
that accepts a database.
Thank you for fiximg my typos and speling mistakes.

[rharding, 2014-03-20 17:03:43]

Thanks for this branch. It looks really nice. 

For your question, I think a short but good way to go would just be to list
dependent bundles when deleting a charm. Maybe link to those pages if they
needed to be deleted as well. 

I don't see the routes updates for the new urls. Am I missing them? Did you not
need to add those?

https://codereview.appspot.com/78220043/diff/20001/charmworld/utils.py
File charmworld/utils.py (right):

https://codereview.appspot.com/78220043/diff/20001/charmworld/utils.py#newcod...
charmworld/utils.py:284: except ElasticHttpNotFoundError:
nice catch

https://codereview.appspot.com/78220043/diff/20001/charmworld/utils.py#newcod...
charmworld/utils.py:316: db.bundles.remove({'_id': bundle.id})
do we have multiple revisions of a bundle? Would this just clear one vs the
entire charm history then?

[rharding, 2014-03-20 17:04:29]

LGTM aside from routes question. The links and bundle warnings would be a good
follow up card if that sounds reasonable.

[bac, 2014-03-20 19:29:55]

Please take a look.

https://codereview.appspot.com/78220043/diff/1/charmworld/jobs/worker.py
File charmworld/jobs/worker.py (right):

https://codereview.appspot.com/78220043/diff/1/charmworld/jobs/worker.py#newc...
charmworld/jobs/worker.py:56: for _, jobs in self.ingest_jobs:
That's mighty compact but it isn't clear to me *exactly* what it does.  The use
of * seems mysterious.

I think what I've got is clearer so I'll keep it.

https://codereview.appspot.com/78220043/diff/20001/charmworld/tests/test_mode...
File charmworld/tests/test_models.py (right):

https://codereview.appspot.com/78220043/diff/20001/charmworld/tests/test_mode...
charmworld/tests/test_models.py:2374: # A convenience constructor is provided
that accepts a database.
your welcome

https://codereview.appspot.com/78220043/diff/20001/charmworld/utils.py
File charmworld/utils.py (right):

https://codereview.appspot.com/78220043/diff/20001/charmworld/utils.py#newcod...
charmworld/utils.py:316: db.bundles.remove({'_id': bundle.id})
We do not have multiple versions of bundles.  So deleting removes it completely.

[matthew.scott, 2014-03-20 20:02:41]

QA Okay!  LGTM