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

Hook model can not support the object it defined primary key with CharField #127

Closed
xuziheng1002 opened this issue Jan 28, 2020 · 6 comments

Comments

@xuziheng1002
Copy link
Contributor

xuziheng1002 commented Jan 28, 2020

I defined a model like below:

class Ticket(BaseModel):
    code = models.CharField(primary_key=True, default=code_generator, max_length=32)
    status = StateField()

the code_generator return string like TC-18442386587087147008

And when I try to approve the ticket to next stage:

ticket.river.status.approve(as_user=user)

but django returns ValueError: invalid literal for int() with base 10: 'TC-18442386587087147008'

I realized that that Hook model was be defind a IntegerField in source code :

object_id = models.PositiveIntegerField(blank=True, null=True)

In this case, I have to change it to:

object_id = models.CharField(blank=True, null=True, max_length=50)

So if you has other good solution, please let me know,thanks.

ziheng.xzh

@javrasya
Copy link
Owner

Hi @xuziheng1002 ,
I don't think you should change the model in the library since it will be very hard to maintain it for you.

Why do you use a custom primary key? You can still have the column code as unique=True instead but not the primary key. Is there really any value of using it like that for you?

@xuziheng1002
Copy link
Contributor Author

xuziheng1002 commented Jan 28, 2020

Thanks for your response,
there are some development specifications on PK field in our company, the one of spec is do not use mysql auto_increment so we use custom PK(snowflakeid) Instead it as usual.

As above if I change object_id in Hook to models.CharField , do you know is that OK in this case?

this is version info in my project:
django-river version:3.1.0
django version:1.11

@javrasya
Copy link
Owner

The first impression that I have on that is that it shouldn't be a problem as long is it matches with the your primary key type. But it will be a manual step to install your app.

@javrasya
Copy link
Owner

I think it should be charfield in the library as well. I am looking into it. Maybe in the next version, you wouldn't need to modify it manually.

@javrasya
Copy link
Owner

There is another bug I discovered. All the tests I have done so far have been with models with integer primary keys. it turns out the core has some problems with the string primary keys. I am working on a fix that I will bundle it with the schema change and hopefully publish it as another version soon (3.1.1).

I think you should wait for the next version @xuziheng1002

@javrasya
Copy link
Owner

javrasya commented Jan 28, 2020

New version (3.1.1) has just been published with the changes that you need @xuziheng1002, can you try it out?

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