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

Use ObjectIdField instead of CharField(24) #13

Closed
wants to merge 1 commit into from

Conversation

WaVEV
Copy link
Collaborator

@WaVEV WaVEV commented Nov 17, 2024

Add ObjectIdField.
The only concern I have is: is convenient to change all charfield(24) to objectId? or keeping in that way for the other test is ok?

This is needed to fix 161

@WaVEV WaVEV requested a review from timgraham November 17, 2024 19:00
@Jibola
Copy link

Jibola commented Nov 25, 2024

The only concern I have is: is convenient to change all charfield(24) to objectId? or keeping in that way for the other test is ok?

I loosely remember there being issues around string comparison before? I'd say try replacing everything with ObjectId and see if that now keeps tests passing.

@timgraham
Copy link
Collaborator

While users could use object_id = ObjectIdField() for GenericRelation, this isn't cross-database compatible (for example, the admin uses LogEntry.object_id = TextField()), and I think we wouldn't promote use of GenericRelation anyway since it involves $lookup.

@timgraham
Copy link
Collaborator

We decided not to make this change since it breaks other tests that store data of other types.

@timgraham timgraham closed this Dec 14, 2024
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.

3 participants