New Advanced Configuration Options #42
Conversation
src/senaite/sync/browser/add.py
Outdated
|
|
||
| url = form.get("url", "") | ||
| if not url.startswith("http"): | ||
| url = "http://{}".format(url) |
There was a problem hiding this comment.
Safety first! : )
src/senaite/sync/browser/add.py
Outdated
| self.add_status_message(message, "error") | ||
| return self.template() | ||
|
|
||
| import_settings = True if form.get("import_settings") == 'on' else False |
There was a problem hiding this comment.
Please, replace it as follows:
import_settings = form.get('import_settings') == 'on'
src/senaite/sync/browser/add.py
Outdated
| unwanted_content_types = [t.strip() for t | ||
| in unwanted_content_types.split(",")] | ||
| unwanted_content_types = filter(lambda ct: ct in portal_types, | ||
| unwanted_content_types) |
There was a problem hiding this comment.
I suggest to reduce a bit the complexity by adding a function to get the content types:
def get_content_types(self, content_types):
if not content_types:
return list()
portal_types = api.get_tool("portal_types")
...
return content_typesSo, you'll only need to do the following here:
prefixable_types = self.get_content_types(form.get("prefixable_types", None))
content_types = self.get_content_types(form.get("content_types", None))
unwanted_content_types = self.get_content_types(format.get("unwanted_content_types", None))Note with this change you'll be able to remove the logic for prefixable_types below.
| type="submit" | ||
| name="fetch" | ||
| i18n:attributes="value" | ||
| value="Save & Fetch"/> |
| <b>Prefix: <span style="font-size: medium" tal:replace="python: view.storage[storage]['configuration'].get('prefix', 'Not defined')"/></b><br><br> | ||
| Settings will <span tal:condition="python: not view.storage[storage]['configuration'].get('import_settings')">NOT</span>, | ||
| Registry will <span tal:condition="python: not view.storage[storage]['configuration'].get('import_registry')">NOT</span>, | ||
| Users will <span tal:condition="python: not view.storage[storage]['configuration'].get('import_users')">NOT</span> be imported.<br><br> |
There was a problem hiding this comment.
Would be nice to create a function (something like def get_storage_setting(self, parameter, section='configuration') in the view to make the retrieval of all these settings easier. A thumb of rule is to always try to keep the tpl as much simpler as possible, with less logic as possible.
src/senaite/sync/browser/views.py
Outdated
| content_types = storage["configuration"].get("content_types", []) | ||
| unwanted_content_types = storage["configuration"].get("unwanted_content_types", []) | ||
| prefix = storage["configuration"].get("prefix", None) | ||
| prefixable_types = storage["configuration"].get("prefixable_types", []) |
There was a problem hiding this comment.
Same function suggested above to be called from the tpl, could be used here to get all these settings in an easier way
src/senaite/sync/browser/views.py
Outdated
| content_types = storage["configuration"].get("content_types", []) | ||
| unwanted_content_types = storage["configuration"].get("unwanted_content_types", []) | ||
| prefix = storage["configuration"].get("prefix", None) | ||
| prefixable_types = storage["configuration"].get("prefixable_types", []) |
src/senaite/sync/importstep.py
Outdated
| if not p_local_uid: | ||
| if hasattr(existing, "UID") and existing.UID(): | ||
| p_local_uid = existing.UID() | ||
| self.sh.update_by_path(p_path, local_uid=p_local_uid) | ||
| self.sh.update_by_remote_path(p_path, local_uid=p_local_uid) |
There was a problem hiding this comment.
Please, add some comments here to make the code more understandable for newbies
src/senaite/sync/utils.py
Outdated
| ret = [t.strip() for t in content_types.split(",") if t] | ||
| ret = filter(lambda ct: ct.lower() in portal_types, ret) | ||
| ret = filter(lambda ct, types=portal_types: ct in types, ret) |
There was a problem hiding this comment.
Just ensure there are no duplicates
ret = list(set(ret))
Current behavior before PR
Desired behavior after PR is merged
/sync) view will only list the Fetched Data and their Actions. New (/sync_add) View will be available to add a new remote.Screenshots
Adding New Remote Form:
Extended Advanced Options:
I confirm I have tested this PR thoroughly and coded it according to PEP8
and Plone's Python styleguide standards.