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

Make request optional in transition_to_state() #10

Open
zupo opened this issue Nov 6, 2015 · 0 comments
Open

Make request optional in transition_to_state() #10

zupo opened this issue Nov 6, 2015 · 0 comments

Comments

@zupo
Copy link

zupo commented Nov 6, 2015

The transition_to_state() method is more or less just a permissions wrapper around the _transition_to_state() method that does the bulk of the work.

The _transition_to_state() has the request parameter optional, while the transition_to_state() does not. I'd like to make the request parameter optional also in transition_to_state() so that I don't have to pass None in application code that does not use the permissions checker.

There are two approaches I see:

  1. Changing the method signature:
-def transition_to_state(self, content, request, to_state, context=None, guards=(), skip_same=True):
+def transition_to_state(self, content, to_state, request=None, context=None, guards=(), skip_same=True):

But this is backwards incompatible and is likely gonna break application code that relies on positioning of to_state and request parameters in transition_to_state() method.

  1. Making to_state optional too, and failing if it is None:
-def transition_to_state(self, content, request, to_state, context=None, guards=(), skip_same=True):
+def transition_to_state(self, content, request=None, to_state=None, context=None, guards=(), skip_same=True):
+    if not to_state:
+        raise KeyError('"to_state" is a required parameter')

Which approach is better? Suggestions for tackling this issue some other way?

@zupo zupo changed the title Make request optional in transition_to_state() Make request optional in transition_to_state() Nov 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant