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

Datetime fields returning type is not consistant. #834

Closed
zombeer opened this issue Dec 8, 2022 · 5 comments
Closed

Datetime fields returning type is not consistant. #834

zombeer opened this issue Dec 8, 2022 · 5 comments
Labels
bug An existing feature is not working as intended

Comments

@zombeer
Copy link

zombeer commented Dec 8, 2022

Describe the bug
created_at and updated_at fields are having type other than schema declared Datetime fields.

In [10]: gs.serialize()
Out[10]: 
{'uuid': 'dd9b77c8-4089-40f2-9be0-3172bc4a00b2',
   ...
 'token_expires_at': '2022-12-08T13:20:06+01:00',
 'history_id': 31960,
 'subscribed': 1,
 'subscription_expires_at': '2022-12-15T11:20:07+01:00',
 'created_at': '2022-12-08T11:55:21+01:00',
 'updated_at': '2022-12-08T12:20:07+01:00',
 'deleted_at': None}

In [11]: type(gs.created_at)
Out[11]: pendulum.datetime.DateTime

In [12]: type(gs.token_expires_at)
Out[12]: datetime.datetime

To Reproduce
Create a datetime field in mysql db.

Expected behavior
I was hoping to get the same type

Screenshots or code snippets

CREATE TABLE "sessions" (
  "uuid" char(36) NOT NULL,
....
  "token_expires_at" datetime NOT NULL,
....
  "subscription_expires_at" datetime DEFAULT NULL,
  "created_at" datetime DEFAULT CURRENT_TIMESTAMP,
  "updated_at" datetime DEFAULT CURRENT_TIMESTAMP,
  "deleted_at" datetime DEFAULT NULL,
....
);

Desktop (please complete the following information):
Ubuntu 22.04.1 LTS

What database are you using?
MySQL(not sure about version),
masonite-orm==2.18.6

@zombeer zombeer added the bug An existing feature is not working as intended label Dec 8, 2022
@zombeer
Copy link
Author

zombeer commented Dec 9, 2022

well.. I've found __dates__ magic field, but I'm really wonder why do I have to explicitly mention that I wan't to cast everything in one type... ? It's kinda really strange solution.

@zombeer
Copy link
Author

zombeer commented Dec 20, 2022

coming back to this issue... still having problems with it...

In [27]: s = SomeModel.first()

In [28]: s.token_expires_at
Out[28]: DateTime(2022, 12, 19, 19, 35, 23, tzinfo=Timezone('Europe/Berlin'))

In [29]: type(s.token_expires_at)
Out[29]: pendulum.datetime.DateTime

In [30]: new_session = SomeModel()

In [31]: new_session.token_expires_at = pendulum.now()

In [32]: new_session.token_expires_at
Out[32]: '2022-12-20 11:06:29'

In [33]: type(new_session.token_expires_at)
Out[33]: str

@zombeer
Copy link
Author

zombeer commented Dec 20, 2022

Why does datetime field can be returned as three different types? - I've seen str, datetime, and pendulum.datetime.DateTime already... should I expect to see unix epoch at some point?

@zombeer
Copy link
Author

zombeer commented Dec 20, 2022

moreover it's being serialized into different formats:

In [31]: new_session.token_expires_at = pendulum.now()

In [32]: new_session.token_expires_at
Out[32]: '2022-12-20 11:06:29'

while existing models serialized with .serialize() gets format:

'token_expires_at': '2022-12-08T13:20:06+01:00',

this is extremely frustrating

@josephmancuso
Copy link
Member

the logic of this is on line 822 of the Model class.

after looking at these examples more closely. most of the examples i believe is how the IDE is representing the classes object as a string.

the only thing that actuallt threw me for a trip was this line

In [30]: new_session = SomeModel()

In [31]: new_session.token_expires_at = pendulum.now()

In [32]: new_session.token_expires_at
Out[32]: '2022-12-20 11:06:29'

this is a bit confusing but its how the model class is converting the datetime to get ready for saving the date to the database (as a string). that part happens.

you can actually modify this behavior in your own application by overriding tis method on your model to this:

    def get_new_datetime_string(self, _datetime=None):
        """
        Given an optional datetime value, constructs and returns a new datetime string.
        If no datetime is specified, returns the current time.

        :rtype: list
        """
        return _datetime

this would not convert the datetime immediately to a string.

this change in the package actually doesn't fail any tests but with that being said i don't feel comfortable making that change right now.
i believe that when setting a date onto a model to be later saved, it should be manipulated before saving it onto the model.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An existing feature is not working as intended
Projects
None yet
Development

No branches or pull requests

2 participants