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

The model form are supported in the formapi and details #5

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

goinnn
Copy link

@goinnn goinnn commented Jun 27, 2013

  1. Now the model form are supported in the formapi.
  2. A simple way to pass the request to your form (request_passed)
  3. If you overwrite the get_form_kwargs method you can pass more parameters to your form
  4. And some details: reorder the imports, change API.xxx to cls.xxx or self.xxx, remove the clean method from APICall, etc

@goinnn
Copy link
Author

goinnn commented Jul 8, 2013

@hannseman Something about this?

@andreif
Copy link
Contributor

andreif commented Jul 8, 2013

@goinnn What about failing tests?

    self._errors[forms.NON_FIELD_ERRORS] = errors
AttributeError: 'module' object has no attribute 'NON_FIELD_ERRORS'

@goinnn
Copy link
Author

goinnn commented Jul 8, 2013

@andreif So sorry I don't see this error

@andreif
Copy link
Contributor

andreif commented Jul 9, 2013

@goinnn Great, thanks! I am not familiar with this project though, so I would wait @hannseman and @carlmarten to comment. However, I feel like some extra tests are required since you are adding new things here.

@goinnn
Copy link
Author

goinnn commented Jul 9, 2013

@andreif Ok, thanks!.

@hannseman or @carlmarten when you see it please tell me something about the tests... if this pull request needs tests, and What are the necessary tests?

@hannseman
Copy link
Contributor

Hey @goinnn! Thanks for the PR. Please add tests for the APIModelCall, like updating first_name and last_name on a django.contrib.auth.User with and test customizing the instance_pk_param.


def action(self, test):
obj = self.save()
return self.serialize_obj(obj)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why you're serializing the object in the form and not letting render_to_json_response handle this? I don't think we should have two ways to serialize data returned from action()

@hannseman
Copy link
Contributor

I'm also thinking about the whole choosing if you want request passed to your APICalls. What we skipped setting request_passed = False and always pass request as a kwarg and do something like this:

class API(FormView):
    def get_form_kwargs(self):
        kwargs = super(API, self).get_form_kwargs()
        form_class = self.get_form_class()
        if self.request:
            kwargs['request'] = self.request
        if isinstance(form_class, APIModelCall):
            kwargs.update(self._get_model_form_kwargs())
        return kwargs
class APICall(forms.Form):
    def __init__(self, request=None, *args, **kwargs):
        super(APICall, self).__init__(*args, **kwargs)
        self.request = request

I might not be thinking of all use cases but this shouldn't hurt backwards compatibility. What do you think?

@Brettbadboy
Copy link

Ineed help

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

Successfully merging this pull request may close these issues.

4 participants