Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dealing with Extra attributes that are not type "string" #1356

Closed
jamlung-ri opened this issue Oct 3, 2022 · 18 comments
Closed

Dealing with Extra attributes that are not type "string" #1356

jamlung-ri opened this issue Oct 3, 2022 · 18 comments
Assignees
Labels
bug Something isn't working documentation-needed ui UI related issue web2 OCL WEB v2

Comments

@jamlung-ri
Copy link
Contributor

jamlung-ri commented Oct 3, 2022

We noticed today that displaying an Extra attribute that is an integer/float was not appearing properly on the TermBrowser. Example: https://app.staging.openconceptlab.org/#/orgs/PIH/sources/PIH/mappings/6004405/ plus see screenshot below.

This might have to do with a broader issue, however. If someone were to use the TermBrowser to assign a value to a numeric field, will the TermBrowser store it as type string by default? If so, that will be an issue that we should address. This extras.sort_weight field should consistently store the value as a float, even if a user edits it in the TermBrowser, or else it will break the OpenMRS process.

One thought: This is a good use case for Jon's previous ideas regarding source-level attributes that all concepts will have. This would allow Extra attributes to be consistently formatted across concepts in that source.

cc: @bmamlin @paynejd

image.png

snyaggarwal added a commit to OpenConceptLab/oclweb2 that referenced this issue Oct 4, 2022
@snyaggarwal
Copy link
Contributor

@jamlung-ri There was a bug in showing numeric values, I have fixed that, (In raw display everything works fine). The int/floats are stored as int/floats and not strings.

@snyaggarwal snyaggarwal added bug Something isn't working web2 OCL WEB v2 ui UI related issue labels Oct 4, 2022
@jamlung-ri
Copy link
Contributor Author

Perfect, this is now looking great. I also tested behavior to respect numeric vs. string values in an Extra attribute, and the field was recognizing numeric vs. string values as expected.

@paynejd paynejd reopened this Oct 5, 2022
@paynejd
Copy link
Member

paynejd commented Oct 5, 2022

Quickly reopening this so that we document the behavior that the API implements for converting strings into typed attributes -- let's make sure that this is part of OCL Docs and then close this out again. In the future, we'll likely allow users to control datatypes directly

@snyaggarwal
Copy link
Contributor

  • API doesn't do any sort of type casting on the values of extras.
  • TermBrowser tries to convert input into JSON, because the field to enter values in forms is a text field. Other than that no conversion happens.
  • ocldev while importing the content from CSV, can convert datatypes bool,int,float,list and json, in the format -- attr:<name>:<datatype>

@jamlung-ri
Copy link
Contributor Author

  • API doesn't do any sort of type casting on the values of extras.
  • TermBrowser tries to convert input into JSON, because the field to enter values in forms is a text field. Other than that no conversion happens.
  • ocldev while importing the content from CSV, can convert datatypes bool,int,float,list and json, in the format -- attr:<name>:<datatype>

@paynejd Where in the OCL Docs website should this info go? Maybe here? https://docs.openconceptlab.org/en/latest/oclapi/apireference/concepts.html#create-or-update-an-extra

Or do we need to separate this out into multiple sections i.e. one for API, one for TermBrowser, and one for OCL Dev Tools?

@jamlung-ri
Copy link
Contributor Author

jamlung-ri commented Oct 25, 2022

Seems like we need a new page for Extra attributes, for the time being at least.

Location: Goes under https://docs.openconceptlab.org/en/latest/oclapi/apireference/index.html

Also include: https://docs.openconceptlab.org/en/latest/oclapi/apireference/concepts.html#create-or-update-an-extra (and any other extras behavior throughout the Docs e.g. Source extras, Collection extras, etc.)

@paynejd
Copy link
Member

paynejd commented Oct 26, 2022

Several OCL resources support extra attributes:

  • Concepts
  • Mappings
  • Sources
  • Collections

@snyaggarwal any resources I'm missing?

@jamlung-ri Our API Reference documentation only includes this under concepts right now. Once we have a good extras doc page, then we can remove it from Concepts and add some language/links/etc. to all of the relevant pages.

@snyaggarwal
Copy link
Contributor

@paynejd Organization and UserProfile also has extras.

@paynejd
Copy link
Member

paynejd commented Oct 27, 2022 via email

@jamlung-ri
Copy link
Contributor Author

jamlung-ri commented Oct 28, 2022

do source version and collection version have extras that are separate from
source and collection?

@snyaggarwal Related to this, I tried to create a version of a source that has an Extra attribute, and the Extra attribute did not carry over to the source version. See HEAD version vs. version v1. Is this a bug?

@snyaggarwal
Copy link
Contributor

@jamlung-ri This was never implemented and copied from OCLAPI v1.
Do we want to copy extras to new versions of repos?

@paynejd
Copy link
Member

paynejd commented Oct 31, 2022

@jamlung-ri How urgent is this -- can we discuss during dev call tomorrow?

@jamlung-ri
Copy link
Contributor Author

First draft of this documentation is here: https://docs.google.com/document/d/1ofPspfktsXVkNaP3PQzEjcJaW9U1i3r5UMuO0Xz2FvU/edit

@snyaggarwal @paynejd Let me know if you want to review before I post it to OCL Docs tomorrow (i.e. Wednesday Nov. 2)

@snyaggarwal
Copy link
Contributor

snyaggarwal commented Nov 2, 2022

  • new versions to copy extras from HEAD
  • data migration for existing versions to copy from HEAD

snyaggarwal added a commit to OpenConceptLab/oclapi2 that referenced this issue Nov 2, 2022
snyaggarwal added a commit to OpenConceptLab/oclapi2 that referenced this issue Nov 2, 2022
@snyaggarwal
Copy link
Contributor

@paynejd @jamlung-ri this is done on QA

@jamlung-ri
Copy link
Contributor Author

The documentation for this has been posted here: https://docs.openconceptlab.org/en/latest/oclapi/apireference/custom_attributes.html

Regarding Extra attribute copying, this appears to be working now. I tested it on some PEPFAR collections e.g. this one, which now have extra attributes in their versions. I was also able to create new versions that successfully got the Extras from HEAD.

@paynejd Anything else here before we move on to Production? I imagine we will need to do the same migration of Extra attributes in Staging and Production, but deferring to @snyaggarwal if that is not the case.

@snyaggarwal
Copy link
Contributor

@jamlung-ri The migration will happen automatically as part of the deployment.

@snyaggarwal
Copy link
Contributor

this is deployed everywhere.
@jamlung-ri Can we close this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation-needed ui UI related issue web2 OCL WEB v2
Projects
None yet
Development

No branches or pull requests

3 participants