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

Question about workflow model design #32

Closed
psychok7 opened this issue Oct 13, 2016 · 8 comments
Closed

Question about workflow model design #32

psychok7 opened this issue Oct 13, 2016 · 8 comments

Comments

@psychok7
Copy link
Contributor

psychok7 commented Oct 13, 2016

@javrasya I trying out django-river, and i noticed that if i do MyModel.objects.all() i get a AttributeError: type object 'MyModel' has no attribute 'objects', but if i do MyModel.workflow.get_queryset().all() it works.

Is this a bug or a design decision?? How does it work in the admin out of the box then, but when i try it manually i run into that error? Would i always have to use the .workflow to access my models manager?

I needed to make this work with Rest Framework and because of this issue i had to override the create method for example and explicitly say instance = ModelClass.workflow.create(**validated_data) instead of the normal instance = ModelClass.objects.create(**validated_data)

Thanks

@javrasya
Copy link
Owner

Sometimes, user may have their own custom manager on objects field. Overriding this field regardlessly is not good for a third party application. This is why I seperated it as different variable as workflow manager. So, this is design desicion as you say.

I don't actually remember the reason exactly. A situation poped up and I decided to change variable name. Now I remember the reason like it is as I mention earlier.

I see your case and I will think about it.

@psychok7
Copy link
Contributor Author

psychok7 commented Oct 13, 2016

@javrasya how do you make it work with django admin out of the box then? They definitely have .objects in it

Anyways nowadays that sites are API driven its important to keep things compatible with Django Rest Framework out of the box as well (at least). If you have entire control of the codebase then .workflow should be fine but with third party apps and all a simple MyModel.objects.... would crash the app. Please revise that then and if you need help testing it let me know.

@javrasya
Copy link
Owner

javrasya commented Oct 13, 2016

I don't actually see the reason you use django-rivermanager for your regular actions like create etc. There must be some other problem about your code because objects manager should still remain in your model. Can you share you model code?

@javrasya
Copy link
Owner

javrasya commented Oct 13, 2016

There are two thing can be done via django-river workflow manager which is an instance of WorkflowObjectManagerwhich are get_objects_waiting_for_approval and get_object_count_waiting_for_approval. So these are not actually the things you want to use with django-restframework.

I mean, if you want to use those APIs of the workflow manager, you already need to use them explicitly.

@psychok7
Copy link
Contributor Author

psychok7 commented Oct 13, 2016

@javrasya this is my model:

class QualificationRequest(models.Model):
    rnal_code = models.CharField(max_length=50, null=True, blank=True)
    name = models.CharField(max_length=200)
    phone = models.CharField(max_length=50)
    email = models.EmailField()

    status = StateField(editable=False)

    created_on = models.DateTimeField(auto_now_add=True)
    updated_on = models.DateTimeField(auto_now=True)

    def __str__(self):
        return self.name

Unless it's something related to this issue #30

@javrasya
Copy link
Owner

javrasya commented Oct 13, 2016

Adding different manager into the model, makes django not to add default one which is objects. I realized that now.

Django admin works well because it detects default manager and works with it actually. Probably django gives it to admin. In django source code;
models/base.py

        if not opts.managers or cls._requires_legacy_default_manager():
            if any(f.name == 'objects' for f in opts.fields):
                raise ValueError(
                    "Model %s must specify a custom Manager, because it has a "
                    "field named 'objects'." % cls.__name__
                )
            manager = Manager()
            manager.auto_created = True
            cls.add_to_class('objects', manager)

Django decides that, there is already a manager in this model and no need to add default one which is objects one. If you are struggle with this issue with djang-restfreamwork, then it is probably doesn't ask django and get default manger unlike django admin.

This may be a problem for django-river as well. Becuase, I didn't actually want to loose objects one. I will work on it and make it still remain with issue #33

Thank you for the report.

@psychok7
Copy link
Contributor Author

@javrasya aha... pure magic ;)

Yeah there should be an "easy" way for you to let .objects being accessible, maybe inherit from the same model manager as .objects and then add you django-river/workflow methods on top of it??

@javrasya
Copy link
Owner

javrasya commented Oct 17, 2016

@psychok7 automatic injection of workfllow manager is removed. If anyone want to use it, it should be used explicitly now. This is best way to do it I think. Please check issue #33

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