Issue 53831: check the true assay name length since we append onto the name when creating the assay domains (ex. "<assay name> Batch Fields") #7150
Conversation
…e name when creating the assay domains (ex. "<assay nam> Batch Fields")
| } | ||
|
|
||
| // Issue 53831: check the true assay name length since we append onto the name when creating the assay domains (ex. "<assay nam> Batch Fields") | ||
| int actualAssayNameLengthMax = 186; |
There was a problem hiding this comment.
Is this the best place for this check? From what I can see, "Assay Design" is only passed as kindName in the call from AssayDomainServiceImpl. Maybe that should do the check there instead, which would also save us from having the kind name hard-coded in multiple places?
Also, perhaps we should just cap this at 150 or similar instead of trying to eke out every possible character. The prior sentence is 110 characters, for example, and would be miserably long as a design name.
There was a problem hiding this comment.
To your first point: I originally had it within AssayDomainServiceImpl but moved it to this location in DomainUtil so that it would be hit while called in the TestDataGenerator.isNameValidRemote() code (which calls property-validateDomainAndFieldNames.api). @labkey-tchad I know you were moving away from that api call to the local test check. Do you know when / if this isNameValidRemote() is still being called?
There was a problem hiding this comment.
To the second item: I totally agree and almost went that route. I can definitely change this to something like 150 chars as the max. Some "round" number would be much more explainable to the users. Anyone have strong feelings on what that number should be? If not, I'll go with 150.
There was a problem hiding this comment.
Currently, isNameValidRemote only gets called if you run the test with webtest.remote.domain.validation=true. Nothing on TeamCity runs that way.
There was a problem hiding this comment.
in that case I will move this code back to AssayDomainServiceImpl
labkey-tchad
changed the title
Rationale
https://www.labkey.org/home/Developer/issues/issues-details.view?issueId=53831
This PR doesn't fix or update anything on the server side to actually allow for assay names of the full 200 characters. This instead adds a check for what is currently the "true assay name max length" at assay creation time.
Note that this definitely feels like a stop gap. If we really want to allow assay names that are 200 characters and not this arbitrary length, we should put that on the backlog and figure out an approach that works for assays but probably applies to all data types so they all have the same name max length.
Related Pull Requests
Changes
Tasks 📍