search

ralsina / aranduka

Automatically exported from code.google.com/p/aranduka
0 stars 1 forks source link

Review of the removal of books #14

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
I implemented the functionality of the "delete" option in the context menu in 
r20ec584eb1. Could you check it out? I'm not sure if the file handling should 
be in the main module, but...

Original issue reported on code.google.com by andresgattinoni on 26 Jan 2011 at 11:24

GoogleCodeExporter commented 9 years ago 
Little fix on rd0563b6f08989f91d8cb367c9ea579266f53fd53

Original comment by andresgattinoni on 26 Jan 2011 at 11:53

GoogleCodeExporter commented 9 years ago 
I think that should move to a methodin the Book class.

Original comment by roberto.alsina on 27 Jan 2011 at 2:33

GoogleCodeExporter commented 9 years ago 
Mmm I agree it should be moved somewhere better. I'm not so keen on having the 
model handling file downloading and unlinking. But for the time being, I could 
overload the delete() method of the Book model to have it delete all the book's 
files and sanitize the Author table.

Original comment by andresgattinoni on 27 Jan 2011 at 2:39

GoogleCodeExporter commented 9 years ago 
I am thinking of the filesystem as just an extension of the database, at least 
when it comes to files that "belong" to a book, that's why I suggested that.

Also, if you put the deletion on the UI layer, then we can't write headless 
scripts that do interesting stuff (like, deduplication, mass conversion, 
whatever) or we have to handle all that stuff on each script.

Original comment by roberto.alsina on 27 Jan 2011 at 2:51

GoogleCodeExporter commented 9 years ago 
Yes, you're right about that. At least with the new downloader module we should 
be able to remove the urllib2 stuff from the model.

Check my changes on r7e959607bc.

Original comment by andresgattinoni on 27 Jan 2011 at 2:57

GoogleCodeExporter commented 9 years ago 
I say merge it :-)

I am not really happy with the downloader in that it has GUI elements, so we 
arestill calling the GUI from the model, but at least it should be fixable by 
just hacking the downloader itself into layers.

Original comment by roberto.alsina on 27 Jan 2011 at 3:20

GoogleCodeExporter commented 9 years ago 
Merged.

I agree, the GUI elements of the downloader shouldn't be called from the model. 
But we shouldn't have the model doing blocking file downloads either.

Original comment by andresgattinoni on 27 Jan 2011 at 3:34