divad1196 / odoo_graphql

Graphql Generic integration in Odoo
Other
19 stars 9 forks source link

Null date fields error out when queried #37

Open kristapsdz-saic opened 1 week ago

kristapsdz-saic commented 1 week ago

What happened?

Fetching a date field with potential null values, such as my_activity_date_deadline from the CRM module, raises GraphQL errors. To reproduce, install a fresh Odoo (version 17) and corresponding odoo_graphql and enable the CRM module with demo data. Then post the following to the GraphQL endpoint in the usual way.

query MyQuery {
  CrmLead {
    my_activity_date_deadline
  }
}

This will produce the following GraphQL error:

{"data":null,"errors":{"message":"'bool' object has no attribute 'toordinal'"}}

Note that requesting other fields will have them returned properly: this only happens with nullable date fields such as the above or date_deadline.

If a format string is passed to the field, it results in a different error:

{"data":null,"errors":{"message":"'bool' object has no attribute 'strftime'"}}

What did you expect?

The value should not error out, consistent with other nullable values. For example, the activity_summary will return a string value for the activity or the boolean false if not set. (Note: this is contrary to the GraphQL introspection documentation, but that's besides the point.) I would expect that nullable date fields would either return false if not set or the requested value, which is a number for ordinal values, string for formatted, etc.

Odoo Version

17

What browsers are you seeing the problem on?

No response

Relevant log output from the server

2024-06-26 15:16:56 odoo-web-1  | 2024-06-26 22:16:56,288 1 CRITICAL odoo odoo.addons.odoo_graphql.graphql_resolver: 'bool' object has no attribute 'toordinal' 
2024-06-26 15:16:56 odoo-web-1  | 2024-06-26 22:16:56,290 1 INFO odoo werkzeug: xxx.xxx.xxx.xxx - - [26/Jun/2024 22:16:56] "POST /graphql HTTP/1.1" 200 - 7 0.017 0.035
divad1196 commented 1 week ago

Hi @kristapsdz-saic and thank you for following the procedure.

I didn't have much time this week and I will take a look deeper into it. Since any field could be nullable (and therefore return False), I was wondering if there couldn't be a generic pre-catch of False value to avoid having it on each transformation function. From what I already saw, this is not ideal and would probably make the code slower.

So you are indeed correct on the resolution as it uses the same check as for datetime. I will conduct some more checks and validate the PR as soon as possible. Thank you for the issue and PR.

divad1196 commented 6 days ago

Hi @kristapsdz-saic ,

As you saw, I applied a fix and added tests. The module should refresh itself in the app store soon (in 1 day at most). I tested it myself and everything seemed fine. Could you confirm?

BTW: I am in the process of backporting this fix up to version 14.0 (16.0 is done already, 15.0 is in progress).