Controlled keywords are persisted inconsistently by Experiment create/update routes

[bencap]

During experiment creation, we attempt to persist keywords by first validating the list and then searching in a case-insensitive manner, see:

mavedb-api/src/mavedb/routers/experiments.py

Lines 308 to 327 in 37659db

	if item_create.keywords:
	all_labels_none = all(k.keyword.label is None for k in item_create.keywords)
	if all_labels_none is False:
	# Users may choose part of keywords from dropdown menu. Remove not chosen keywords from the list.
	filtered_keywords = list(filter(lambda k: k.keyword.label is not None, item_create.keywords))
	try:
	validate_keyword_list(filtered_keywords)
	except ValidationError as e:
	raise HTTPException(status_code=422, detail=str(e))
	for upload_keyword in filtered_keywords:
	try:
	description = upload_keyword.description
	controlled_keyword = search_keyword(db, upload_keyword.keyword.key, upload_keyword.keyword.label)
	experiment_controlled_keyword = ExperimentControlledKeywordAssociation(
	controlled_keyword=controlled_keyword,
	description=description,
	)
	keywords.append(experiment_controlled_keyword)
	except ValueError as e:
	raise HTTPException(status_code=422, detail=str(e))

During experiment updates, we validate keywords but then attempt to set them using a class method on the DB object which is case-sensitive:

mavedb-api/src/mavedb/routers/experiments.py

Lines 461 to 473 in 37659db

	if item_update.keywords:
	all_labels_none = all(k.keyword.label is None for k in item_update.keywords)
	if all_labels_none is False:
	# Users may choose part of keywords from dropdown menu. Remove not chosen keywords from the list.
	filtered_keywords = list(filter(lambda k: k.keyword.label is not None, item_update.keywords))
	try:
	validate_keyword_list(filtered_keywords)
	except ValidationError as e:
	raise HTTPException(status_code=422, detail=str(e))
	try:
	await item.set_keywords(db, filtered_keywords)
	except Exception as e:
	raise HTTPException(status_code=500, detail=f"Invalid keywords: {str(e)}")

I think the case in-sensitive approach does make sense, but we should update keywords in the same manner during object creation and updating, and also more robustly test these routes on differently cased keywords to ensure the behavior is consistent in both routes.

[EstelleDa]

I leave my comment here as well. I changed the UI side key: 'Delivery method' to key: 'Delivery Method'. But I don’t understand why it works for creating an experiment, but throws an error in editing the experiment. Changing it back won't get this error.

[bencap]

Yeah I think that when we make the call to search_keyword we are looking at a case-insensitive version of the keyword, whereas with item.set_keywords we don't lower-case the keyword. We probably want to standardize on something, whether that is using search_keyword in both places or set_keywords in both places.

I prefer the case-insensitive matching here-- I don't think there's any question a user would be attempting to match Delivery Method even if they sent us delivery method.

[EstelleDa]

I prefer the case-insensitive matching too. That's why I used the search_keyword here, but I can't remember why it's set_keywords in the updating experiment. I modified the code of the updating experiment to the search_keyword, but it didn't work. I'll keep trying it today.