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

Provide support for contextually-readonly properties, and odata services which don't follow Foreign Key BINDs #5

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

jonnerd154
Copy link

@jonnerd154 jonnerd154 commented Oct 9, 2024

These tweaks solve issues with support for SAP Business One's Service Layer. All edits welcome. I don't believe these changes have adverse impacts for other services, and are likely workarounds for problems in SAP B1. They solved my problems, but YMMV - more testing encouraged.

@jonnerd154 jonnerd154 changed the title Provide support for contextually-readonly properties, and service which don't follow Foreign Key BINDs Provide support for contextually-readonly properties, and odata services which don't follow Foreign Key BINDs Oct 9, 2024
@eblis
Copy link
Owner

eblis commented Oct 10, 2024

I can't test it with SAP as I don't have access to it.
I made some changes, added explanation about the new field and also made it not filter out everything when not in use.

Can you please test and see if it works for you?

@eblis
Copy link
Owner

eblis commented Oct 10, 2024

Can you please check if the properties SAP fails for are marked as Nullable=false ?

@jonnerd154
Copy link
Author

jonnerd154 commented Oct 10, 2024

Your changes are great: type hints, documentation, improve code reuse, and the False case is smart. Thank you.

I pushed another commit that prevents calling .len() on a bool when omit_null_props=False.

Regarding the Nullable=False fields, unfortunately that's not the issue.

For example, the Orders entity has its CardCode property (the foreign key to BusinessPartner aka "Customer") spec'd as Nullable=False, and that validation works correctly: if it's null, the service returns an error.

The problem is that I get an error if I send property InterimType using it's initialized value as None/null. In the metadata spec, InterimType IS Nullable. To make matters more frustrating, if I get() the freshly inserted entity instance, InterimType is null; I'm not trying to change it on insert, and no change would have occurred, but the mere attempt at asserting its value, even if the value provided is the same as the default, triggers the error.

I think this could be best solved by improving the behavior of properties' __set__() and is_dirty() checking. If the client has attempted to set the value of a property, even if setting it to None, it should be included in the the request. If it's untouched, it is omitted from the request. This keeps the interface clean, and eliminates the need to manually list the offending properties or blindly omit all None values. Some people might actually want to set a value to Null explicitly.

Thoughts? I'd be happy to take a crack at it if you agree.

@eblis
Copy link
Owner

eblis commented Oct 14, 2024

Sounds good. If you have the time go ahead.

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.

2 participants