Deleting non-existent iPaper document throws error
Reported by Matt Darby | April 7th, 2009 @ 06:11 PM
If one tries to delete an has_ipaper_and_uses model
and does not have an ipaper document set on it, an exception is
thrown. This is because the
ScribdFu::load_ipaper_document method throws an
exception rather than returning nil (causing, in turn,
the model method ipaper_document to do the same, even
though it says it returns null).
There are different ways to fix this issue at different levels.
The simplest is to make ScribdFu::load_ipaper_document
return nil. Or you could change the condition in
destroy_ipaper_document to be if
ipaper_id. Depends on the broader implications.
Comments and changes to this ticket
-
Matt Darby April 7th, 2009 @ 06:35 PM
Is it better to just return
niland log the rescue from Scribd?I'm leaning towards keeping the
ScribdFuErrorin this circumstance. I think the end programmer should rescue @ScribdFuError@, as the rscribd gem will throw an error its self if the document isn't found anyway. -
shai.shefer (at gmail) April 7th, 2009 @ 07:13 PM
Does the same thing happen when using attachment_fu? If so then I would keep it the same way if only for consistency and to make it easier for you. :)
My initial thought was actually to return nil, because if it's not set why bother but I think keeping the error is the right way to go. If the user explicitly sets a model to use scribd_fu they want everything uploaded to scribd. I know I'd like to know the error existed and try to track down why rather than change it to nil.
I think in the end, if someone wants all their documents upload to scribd they should be concerned if ipaper fields were no filled in. If they're not, maybe this gem is not the right solution for their app?
-
Matt Darby April 7th, 2009 @ 07:18 PM
- State changed from “new” to “wontfix”
Yep, this issue is base plugin independent. It's only concern is ScribdFu finding the associated iPaper document via Scribd's API. If the document isn't found, then an error should be thrown. Seems simple enough. I'm thinking this should remain as an error.
-
Paul Canavese April 12th, 2009 @ 05:07 PM
- Assigned user set to “Matt Darby”
But what if content type is not scribdable (is not a supported content type)? Then the model is created without an ipaper id. When an attempt is made to delete the model, it will fail.
-
Matt Darby April 12th, 2009 @ 07:23 PM
- State changed from “wontfix” to “resolved”
Good point -- I never thought of that angle. I removed the begin/rescue block from @ScribdFu::load_ipaper_document@, and it will now return
nilwhen the iPaper document doesn't exist.2.0.4 will contain this fix.
Please Sign in or create a free account to add a new ticket.
With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.
Create new ticket
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
A Ruby on Rails plugin that streamlines interaction with Scribd.com’s iPaper service, and works along side of either Attachment_fu or Paperclip.
Matt Darby
Matt Darby