Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

404 for actions returned by get_object_actions() but not in objectactions #25

Open
mjog opened this issue Sep 2, 2014 · 5 comments
Open

Comments

@mjog
Copy link

mjog commented Sep 2, 2014

Currently if an admin view overrides get_object_actions() and returns an action not contained in the objectactions view's attr, then invoking the action from the admin interface results in a 404.

This occurs because BaseDjangoObjectActions. get_tool_urls uses self. objectactions, not self.get_object_actions().

It seems this might be unavoidable, but is unexpected since it's not documented, and annoying if you're trying to create a base/mixin admin view class that use get_object_actions() to provide actions in addition to subclasses that provide their own.

@crccheck
Copy link
Owner

crccheck commented Sep 2, 2014

Sounds annoying. The example project's admin is really... too simple right now. I rely on it to document all the ways object actions could be used. Before tackling this, I (or some helpful person) would need to flesh out the example admin to recreate this scenario.

@crccheck crccheck added the bug label Sep 2, 2014
@mjog
Copy link
Author

mjog commented Sep 3, 2014

I could look at fleshing out the example admin a bit, but would you be interested in merging a more elaborate fix for this? I see three possible approaches:

  1. Leave it as is, and document it
  2. Add a new method, say get_all_object_actions(self) and call that from get_urls(), and document that
  3. Call get_object_actions() from get_urls() with zero/null args, and document that

I'm not a fan of the first option, since having to both specify the attribute value and override the method is inconsistent with the rest of ModelAdmin's customisation hooks, there's no error at runtime until you hit the 404, and implementing get_object_actions() means removing disabled actions from the list if you chain up to super's implementation (as all good classes should) rather than adding enabled actions, leading to a lot of bug-prone double negatives in the logic.

The second option is an improvement since it's more consistent with how ModelAdmin's other customisation hooks work, but you will get either missing actions or the mysterious 404 if you forget to implement both get_all_object_actions and get_object_actions until you go back and reads the docs and do it properly.

The third option sucks because it is also inconsistent with ModelAdmin's customisation hooks, but at least implementers will get an error thrown when it is called by get_urls, so they will know to go look for the problem, and you only have to implement one method, rather than fiddle with two things.

So, I'd go with the third. Thoughts?

@crccheck
Copy link
Owner

crccheck commented Sep 4, 2014

I tried thinking through this today, and I have one idea in my head that's not the same as what you're thinking. I haven't got the imagination right now to visualize what you're describing.

@psalonen
Copy link

psalonen commented Feb 6, 2015

Are there any fixes available for this issue? I want to use these actions but i need the runtime customization of available actions per object.

@architux
Copy link
Contributor

I've faced the same issue just now.

get_object_actions() method works if every action that it would conditionally enable or disable is listed above in change_actions or changelist_actions attribute. You should pass there everything that you may or may not use later.

I don't know whether it should be fixed or not, but such a behavior should be mentioned in documentation to avoid same questions in the future.

@crccheck crccheck added the to do label Apr 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants