Allow SEL to load experiments with protocols that match existing case-insensitive

[jmcneil86043]

When loading an experiment, ACAS searches for an existing protocol to associate. This feature would do a case-insensitive search for the protocol name. This new feature should be enabled with a configuration that defaults to off so existing systems behaviors don’t change on upgrade

Expected Behavior

When an experiment SEL file is dry-run, if the protocol name provided matches an existing protocol name, but with different capitalization, a warning is issued. If the file is uploaded, then the new experiment is associated with the protocol it found with the alternate capitalization. If there are two protocols in the system that match with different cases, then an error is returned

Current Behavior

The standard no matching protocol behavior is followed for protocol name that have different capitalization

[brianbolt]

This is a great new feature but I don't think this should be a feature flag, it sounds like it should be the default. It's up to the user to head to the warning of uploading to the wrong protocol.

[jmcneil86043]

Getting serious about implementation. Here is my analysis

• in racas api.R
  • Line 78 checkExistance()
    • calls getEntity which calls persistence
      • protocols?FindByProtocolName&protocolName=
        • src_main_java_com_labsynch_labseer_api/ApiProtocolController.java 379
          • src_main_java_com_labsynch_labseer_domain/Protocol.java 132
            • src_main_java_com_labsynch_labseer_domain/ProtocolLabel.java 174
  • There is a ProtocolLabel.findProtocolLabelsByNameLike
    • does it have an API?
      • not what we want, the only use of this label function is in listJsonCodeTable
• Options
  • 1. Change current protocols?FindByProtocolNam API behavior
    • src_main_java_com_labsynch_labseer_domain/ProtocolLabel.java 180
      • from WHERE o.labelText = :labelText
      • to WHERE LOWER(o.labelText) = LOWER(:labelText)
    • Pros: simple change "fixes" it everywhere
    • Cons: "fixes" it everywhere leading to unexpected consequences like an API callers expects to get one protocol, but get two
  • 2. Make new Protocol API and call from racas
    • signature could be: protocols?FindByProtocolNameLike&protocolName=
    • Pros
      • api.R checkExistence() already handle multiple matches gracefully
    • Cons
      • need to change two functions in two modules
• Recommendation
  • option 2

@brianbolt @bffrost What is your advice?

[bffrost]

@jmcneil86043 I like option 2. It seems safer for the reasons you outlined.

[philippcheung]

Just making I understand -- do we need to implement a new r-acas method and a roo method

New method in r-acas

New method in roo -- APIProtocolController.java

New method in roo -- Protocol.java

Looks like ProtocolLabel.java already has a implementation of like

Finally, let me know if you'd rather add new "searchType" to the method instead, where the default is exact, but options can include "exact","like"

[jmcneil86043]

Hi @philippcheung I don't have a qualified opinion about new have API versus qualified. It looks to me like a new API would match prior practice (as you pointed out, for example, in ProtocolLabel.java

I'm not sure you need a new R method or if you should change the behavior of the current one. I guess a new function is nice parallel construction, so easy to read. In either case you also need to change racas api.R Line 78 checkExistance()

@brianbolt thoughts?

[brianbolt]

I think you should just change the behavior of the current racas function. I just think the other one would be unused if we duplicated it and pointed it at the other java route.