Migrated instrument locations to Dexterity #1705
Conversation
…com/senaite/senaite.core into convert-storagelocations-to-dexterity
| obj = _createObjectByType(portal_type, container, tmp_id) | ||
| obj = _createObjectByType(portal_type, container, id) | ||
| obj.processForm() | ||
| obj.edit(title=title, **kwargs) |
There was a problem hiding this comment.
Wouldn't be necessary to notify the modification explicitly?
notify(ObjectModifiedEvent(obj))I recall there are some subscribers for ObjectModifiedEvent (for Contact, Batch, ...)
Probably for both ATs and DX.
Also, what if we do a noLongerProvides(IAuditLog, obj) before the edit and an alsoProvides(IAuditLog, obj) afterwards?
There was a problem hiding this comment.
In my opinion this modification event was always wrong in that function, because we are creating an object and not modifying it. Therefore, I think it is better to handle that internally as just one transaction.
There was a problem hiding this comment.
Understand, however, we'll need to keep this in mind. I think Batch type is the only one that might be affected by this change in core, but not sure atm about the scope of this change in other add-ons (e.g. jsonapi)
| "The place where the instrument is located in the laboratory")) | ||
|
|
||
| self.show_select_column = True | ||
| self.pagesize = 25 |
There was a problem hiding this comment.
I wouldn't set the pagesize explicitly. I think is 50 by default, which is fine imo
There was a problem hiding this comment.
The pagesize is set to 25 for all of the controlpanel listings and I would like to keep them consistent for now.
Maybe we can make a setup option and refactor all of them in another PR?
There was a problem hiding this comment.
Sounds a good idea. Let's do it in another PR then
| url = obj.absolute_url() | ||
|
|
||
| item["replace"]["Title"] = get_link(url, value=title) | ||
| item["Description"] = description |
There was a problem hiding this comment.
I would simplify to:
def folderitem(self, obj, item, index):
item["replace"]["Title"] = get_link_for(obj)
item["Description"] = api.get_description(obj)| @@ -8,11 +8,17 @@ | |||
| <bindings> | |||
|
|
|||
| <!-- Bindings to "senaite_one_state_workflow" --> | |||
| <type type_id="IstrumentLocations"> | |||
There was a problem hiding this comment.
Typo: "IstrumentLocations" --> "InstrumentLocations"
| @@ -54,6 +54,7 @@ Client's folder: | |||
| If the client is assigned on creation, same behavior as before: | |||
|
|
|||
| >>> batch = api.create(portal.batches, "Batch", Client=client) | |||
| >>> modified(batch) | |||
There was a problem hiding this comment.
This wouldn't be necessary, see my previous comment
Description of the issue/feature this PR addresses
This PR converts existing AT based instrument locations to Dexterity.
Current behavior before PR
Desired behavior after PR is merged
--
I confirm I have tested this PR thoroughly and coded it according to PEP8
and Plone's Python styleguide standards.