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

Prioritize getters instead of fieldnames when getting values from instance #19

Merged
merged 7 commits into from
Feb 9, 2024

Conversation

xispa
Copy link
Member

@xispa xispa commented Feb 9, 2024

Description of the issue/feature this PR addresses

This Pull Request fixes a bug introduced with senaite/senaite.core#2471 because of the use of properties (e.g. Manager to let supermodel and impress overcome the fact that with dexterity, name fields are in lower case. However, when calling the property with supermodel, the system returns the object directly, instead of the function:

> /home/senaite/zinstance/src/senaite.app.supermodel/src/senaite/app/supermodel/model.py(232)
 214         def get(self, name, default=None):                                                                                       
 215             # Internal lookup in the data dict                                                                                   
 216             value = self.data.get(name, _marker)                                                                                 
 217                                                                                                                                  
 218             # Return the value immediately                                                                                       
 219             if value is not _marker:                                                                                             
 220                 return self.data[name]                                                                                           
 221                                                                                                                                  
 222             # Field lookup on the instance                                                                                       
 223             field = self.get_field(name)                                                                                         
 224                                                                                                                                  
 225             if field is None:                                                                                                    
 226                 # expose non-private members of the instance/brain to have access                                                
 227                 # to e.g. self.absolute_url (function object) or self.review_state                                               
 228                 if not name.startswith("_") or not name.startswith("__"):                                                        
 229                     # check if the instance contains this attribute                                                              
 230                     instance = self.instance                                                                                     
 231                     instance_value = getattr(instance, name, _marker)                                                            
 232  ->                 if instance_value is not _marker:                                                                            
 233                         return instance_value    
(Pdb++) instance
<Department at /senaite/setup/departments/department-1>
(Pdb++) getattr(instance, "Manager")
<LabContact at /senaite/bika_setup/bika_labcontacts/labcontact-1 used for /senaite/setup/departments/department-1>
(Pdb++) getattr(instance, "getManager")
<bound method Department.getManager of <Department at /senaite/setup/departments/department-1>>

When Department object was an Archetype, self.get_field(name) at line 223 would return the accessor to the field Manager, so the output of process_value would be returned instead.

Since Departmen is no longer AT, but a DX, there is no field Manager and calling model.Manager returns the object instead of the expected SuperModel, which leads to the following traceback in senaite.impress:

Error: Fullname

    Expression: "manager/Fullname"
    Filename: ... /senaite/impress/analysisrequest/templates/signatures.pt
    Location: (line 19: col 35)
    Source:
    ^^^^^^^^^^^^^^^^
    Expression: "python:view.render_signatures(context, **options)"
    Filename: ... ss/src/senaite/impress/templates/reports/MultiDefault.pt
    Location: (line 37: col 28)
    Source: ... ucture python:view.render_signatures(context, **options)" />
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Therefore, this pull request ensures that functions are returned as-is, while non-functions are processed as expected.

Linked issue: senaite/senaite.impress#148

Current behavior before PR

Traceback when trying to publish a results report because the system expects a Supermodel to be returned by model.Manager at senaite.impress.analysisrequest.model.managers

Desired behavior after PR is merged

No traceback. Report is published correctly

--
I confirm I have tested this PR thoroughly and coded it according to PEP8
and Plone's Python styleguide standards.

@xispa xispa changed the title Process values returned from instance unless function Process values returned from instance except when a function Feb 9, 2024
@xispa xispa requested a review from ramonski February 9, 2024 11:19
@xispa xispa changed the title Process values returned from instance except when a function Support for Dexterity's non-CamelCase fieldnames Feb 9, 2024
@xispa xispa changed the title Support for Dexterity's non-CamelCase fieldnames Prioritize getters instead of fieldnames when getting values from instance Feb 9, 2024
Copy link
Contributor

@ramonski ramonski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the improvements!

@ramonski ramonski merged commit 5cacdbf into 2.x Feb 9, 2024
@ramonski ramonski deleted the fix-property-calls branch February 9, 2024 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants