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

Instantiating Serializer makes unexpected API call #4

Open
hugorodgerbrown opened this issue Jan 29, 2016 · 1 comment
Open

Instantiating Serializer makes unexpected API call #4

hugorodgerbrown opened this issue Jan 29, 2016 · 1 comment

Comments

@hugorodgerbrown
Copy link
Contributor

The ContactModelSerializer and DealModelSerializer classes both include this line in the field validation method:

if field_name == 'owner_id' and field_value not in base_helpers.get_user_ids():
    return False

The call to base_helpers.get_user_ids() makes a network API call, which is not what you would expect from object instantiation. We need a way to either remove this until the synchronisation (it makes more sense to fail on sync with 'user not found' that on object instantiation), or at the very least to allow clients to force some kind of lazy validation.

The use case that highlighted this was trying to display the object JSON in the Django admin view, ironically in order to assist with debugging of synchronisation issues. If I want to load in an object, and serialize it, without pushing to Base, how can I do that?

@stevejalim
Copy link

Code in question:

https://github.com/yunojuno/django-basecrm/blob/master/django_basecrm/serializers.py#L177

    def _validate_field(self, field_name, field_value):
        if field_value is None:
            return True

        if field_name == 'owner_id' and field_value not in base_helpers.get_user_ids():
            return False

        return super(ContactModelSerializer, self)._validate_field(field_name, field_value)

Which is called via https://github.com/yunojuno/django-basecrm/blob/master/django_basecrm/serializers.py#L177

    def _get_value(self, field_name):
        """
        Gets the value of the field to be assigned to this instance.
        First sees whether the field has a get_X function (where X is the BaseCRM field name) and if
        so, runs it to derive the value from there.
        If that doesn't exist, we check whether the value is specified on this class and if so,
        assign that (calling the method if applicable) -- if not, we see whether the instance has
        an identically-named field and we fall back to the value of that.
        """
        ...

        # confirm that whatever we've ended up with is safe to set
        if self._validate_field(field_name, val):
            return val
        else:
            return None

I think making self._validate_field() an optional/toggleable call via a settings - with the assumption (yes, I know) that val is a legitimate one

        # confirm that whatever we've ended up with is safe to set
        if (
                settings.BASECRM_DO_NOT_VALIDATE_ON_INSTANTIATION 
                or self._validate_field(field_name, val
        ):
            return val
        else:
            return None

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

2 participants