-
Notifications
You must be signed in to change notification settings - Fork 7
Description
Current implementation details
Currently when the dsc ls receives search_terms it separates fields and values by looking for =
The CLI handler (Click 8.x framework) is configured to accept any number of arguments and does NOT require them to be surrounded by quotes. It receives any args that are not part of an option already, as a tuple. E.g this is valid:
dsc ls --order-by price --reverse --limit 10 word1 word2 word3
and the tuple will look like this:
("word1", "word2" "word3"). It's passed to the command function in a variable called search_terms
There is one special case handled that grabs all parts of the search_terms that are standalone, for example album name (no =) and creates a title=%album%name% out of that to be successfully be parsed by furhter processing that require a field name to work.
Feature suggestion
There is a caveat with this solution an SQL select containing LIKE album_title = "%album%name" would find an album named "album name" but not one named "name album".
This could be improved by producing an SQL select that looks similar to this: LIKE album_title = "%album%" AND album_title = "%name%"
That way, also a release titled "name album" would be found.
Also, this change would pave the way for handling additional - let,s call them - auto-search fields, for example artist and catalog number
Relevant code parts (current implementation)
CLI level
Line 16 in 9740f83
| @click.argument("search_terms", metavar="SEARCH_TERMS", nargs=-1) |
Lines 30 to 31 in 9740f83
| def ls_cmd(helper, search_terms, order_by, reverse, sales_extra, limit): | |
| """Searches and lists collection items. |
Lines 51 to 54 in 9740f83
| try: # Catch when `=` character is missing and assume title column | |
| search_key_value = coll_ctrl.prepare_key_value_search(query=search_terms) | |
| except ValueError as error: | |
| coll_ctrl.cli.p(error) |
Happening here is: Separating keys and values by delimiter = and enriching standalone arguments with a key, currently hardcoded to title:
discodos/discodos/ctrl/collection.py
Lines 1224 to 1244 in 9740f83
| def prepare_key_value_search(self, query): | |
| """Returns a dictionary from space-delimited key=value pairs.""" | |
| kv = {} | |
| non_kv = [] | |
| for item in query: | |
| if "=" in item: | |
| key, value = item.split("=") | |
| kv[key] = value | |
| else: | |
| non_kv.append(item) | |
| if non_kv: | |
| if not kv: | |
| kv = {"title": "%".join(non_kv)} | |
| elif "title" in kv: | |
| kv_title = kv["title"].replace(" ", "%") | |
| non_kv_terms = "%".join(non_kv) | |
| kv["title"] = f"{kv_title}%{non_kv_terms}" | |
| else: | |
| kv["title"] = "%".join(non_kv) | |
| return kv |
The final result for a query like this: dsc ls word1 status=draft word2 word3
is a dictionary (kv) that looks like this:
{
'status': 'draft',
'title': 'word1%word2%word3',
}which is then finally passed to the controller level:
Lines 56 to 62 in 9740f83
| if user.conf.enable_tui: | |
| coll_ctrl.tui_ls_releases( | |
| search_key_value, | |
| orderby=order_by, | |
| reverse_order=reverse, | |
| sales_extra=sales_extra, | |
| limit=limit, |
Controller level
a proper key/value dictionary is expected here:
discodos/discodos/ctrl/collection.py
Lines 1246 to 1249 in 9740f83
| def tui_ls_releases( | |
| self, search_terms, orderby=None, reverse_order=False, sales_extra=False, limit=None | |
| ): | |
| """search_terms is a key value dict: eg: d_artist: artistname""" |
Model/Database abstraction level
discodos/discodos/model/collection.py
Lines 840 to 843 in 9740f83
| def key_value_search_releases( | |
| self, search_key_value=None, orderby=None, filter_cols=None, | |
| custom_fields=None, reverse_order=False, sales_extra=False, limit=None, | |
| ): |
Proposed solution
The first step in improving all this as pointed out in Feature suggestion would be to refactor prepare_key_value_search() to produce a dictionary like this::
{
'status': 'draft',
'title': ['word1', 'word2', 'word3'],
}That way the database query function receiving the dictionary could easily interpret this to handle the title list as AND-concatenated SQL select parts.