search

msupply-foundation / mobile

Open source mobile app for medical inventory control
http://msupply.org.nz/mobile
Other
42 stars 27 forks source link

Link name_notes with JSON Schema version #4639

Closed kat-ms closed 2 years ago

kat-ms commented 2 years ago

Is your feature request related to a problem? Please describe.

Required for #4624

Describe the solution you'd like

Currently there is no link between which JSON schema version was used to generate a name note. This could lead to problems when trying to edit a name_note that was created with an older version of a JSON Schema.

This also means that when JSON schemas are updated on the server, it should be a new version with the version number incremented, and not a replacement of the existing JSON Schema.

Implementation

Describe alternatives you've considered

A clear and concise description of any alternative solutions or features you've considered.

Additional context

Add any other context or screenshots about the feature request here.

anildahalsussol commented 2 years ago

@adamdewey: I am not sure but while checking formSchema I don't find old schema's so I believe we always overwrite the old one with a new one. If that's the thing then it can be tough to validate properly. Suppose we want to edit an old schema and it will fail validation as we don't have that schema in the server anymore. I may be wrong as well. So we are planning if we can create a new one rather than overlapping the old one and give a certain name like version1 v1... Can you please clarify on this one and then we can come up with one agreed solution. Thanks 🙏

adamdewey commented 2 years ago

@anildahalsussol - yes, that is what happened historically. We would just store the latest version of the schema on the server.

Perhaps now we need to do 3 things:

What do you think about the last point @katherine-sussol - I guess there will be quite a few old name_notes that don't match up with the current schema. I wonder how we should handle these...

anildahalsussol commented 2 years ago

@adamdewey @katherine-sussol : Suppose a patient uses the v1 form schema and on request, we created a new form schema v2 which will be available on a tablet. Now we keep both on a tablet for validation but an old patient with a name note in the old schema will always open that same old schema which I don't think the client wants. So I think while opening old name notes rather than with old UI if we can make the old name note compatible with the new schema then it can be a good solution. And also I don't think we can open multiple name notes from the tablet until it's in future plan.

adamdewey commented 2 years ago

Hey @anildahalsussol - are you thinking to update the data of the old name notes or to make the new schema able to detect the format of the old name notes and leave the old data untouched?

I agree that it would be better for the user's experience for Mobile to display the current new schema and to indicate if some data is missing from the old name notes.

anildahalsussol commented 2 years ago

Hey @anildahalsussol - are you thinking to update the data of the old name notes or to make the new schema able to detect the format of the old name notes and leave the old data untouched?

I agree that it would be better for the user's experience for Mobile to display the current new schema and to indicate if some data is missing from the old name notes.

I was thinking if we compare old and new schema. Like what i found is in old schema patientID is inside the demographic and in new schema patientID is a separate keyword. So what I was thinking if we can reformat those data so that while opening old json in new schema we can see all the field that are available in new schema and leave extra data as it is which we dont display in tablet.

adamdewey commented 2 years ago

ah, I was hoping we could avoid a data script if possible...

I am looking at how we handle viewing the old schema at the moment (using mSupply Mobile V8.5.0-rc2 with Kat's new changes).

I think we have 2 'kinds' of old schema:

For the vaccination events that were inserted using an earlier version of mSupply Mobile, it seems like the missing field is just left blank:

image

My hope would be that the user could edit this field and add a date and then the new schema would be used going forward - how possible is that @katherine-sussol ?

For the vaccination events that were imported in June 2021, there does seem to be more of an issue with reading the data:

image

Maybe we need to have a meeting @anildahalsussol, @katherine-sussol to discuss options?

kat-ms commented 2 years ago

I couldn't find any Kiribati vaccination events that had data in 'weird' formats that would 'break' the schema (e.g. as per the example above, the updates I saw were additional fields that were added as Kiribati's vaccination rollout progressed that didn't exist in older versions of the schema). In these cases, the field would be blank, and (when editable) can be populated before saving (if the field is mandatory, users will be forced to enter before saving).

The missing PCD is another story. I didn't realise there were imports done where a vaccination event was created without a corresponding PCD 🙀 . I feel like no data is accurate if there wasn't a PCD in the system at the time of vax? Or do we want to show a blank one that we let them fill in as part of the edit flow?

Happy to do a meeting!

adamdewey commented 2 years ago

Yeah I think we just imported the following:

name nationality occupation transaction transline item_line medicine_administrator name_store_join

See https://github.com/sussol/msupply/issues/9278

So guess we wouldn't have created a PCD name_note - @anildahalsussol may remember better?

"Or do we want to show a blank one that we let them fill in?"

If there's a way to do this that would be the ideal solution!

Cheers

On Mon, May 30, 2022 at 3:49 PM Kat @.***> wrote:

I couldn't find any Kiribati vaccination events that had data in 'weird' formats that would 'break' the schema (e.g. as per the example above, the updates I saw were additional fields that were added as Kiribati's vaccination rollout progressed that didn't exist in older versions of the schema). In these cases, the field would be blank, and (when editable) can be populated before saving (if the field is mandatory, users will be forced to enter before saving).

The missing PCD is another story. I didn't realise there was an import done where a vaccination event was created without a corresponding PCD 🙀 . I feel like no data is accurate if there wasn't a PCD in the system at the time of vax? Or do we want to show a blank one that we let them fill in?

Happy to do a meeting!

— Reply to this email directly, view it on GitHub https://github.com/openmsupply/mobile/issues/4639#issuecomment-1140661368, or unsubscribe https://github.com/notifications/unsubscribe-auth/AETMRGMNWH4JKEKGLGYTM5DVMQ3BBANCNFSM5RS65WLA . You are receiving this because you were mentioned.Message ID: @.***>

FlorenceChashchina commented 2 years ago

@anildahalsussol Hey Anil just checking if you're working on this issue still or is it available for pick up

anildahalsussol commented 2 years ago

@anildahalsussol Hey Anil just checking if you're working on this issue still or is it available for pick up

its available for pick up.

anildahalsussol commented 2 years ago

@adamdewey @chastichno @sworup : I think this needs to be done before we update the JSON schema for school. As we want to trace all the schema that we create from now onwards.

sworup commented 2 years ago

@chastichno @adamdewey @anildahalsussol this looks like a lengthy task to do and most importantly to test. If we have to do this in current phase then yes it need to be started right away.

adamdewey commented 2 years ago

@anildahalsussol - unfortunately Kiribati need the school changes asap as they are currently vaccinating people with the Pfizer vaccine for their booster campaign, so I don't think we should wait for the schema tracing for that ticket..

adamdewey commented 2 years ago

@sworup - yes, if we need this to complete #4624 then we should have to do it in the current phase, I agree

FlorenceChashchina commented 2 years ago

@adamdewey @sworup I have a question about how to use new version schemas.

Let's assume we had a patient survey schema with version "1". It was filled in for each patient, used during vaccinations etc. Then, we add a survey schema with version "2", which is different from version "1". What should happen now - should all the patient survey schemas become version 2? What if it's very different? The data would not be valid if the schema was changed a lot.

sworup commented 2 years ago

@adamdewey @sworup I have a question about how to use new version schemas.

Let's assume we had a patient survey schema with version "1". It was filled in for each patient, used during vaccinations etc. Then, we add a survey schema with version "2", which is different from version "1". What should happen now - should all the patient survey schemas become version 2? What if it's very different? The data would not be valid if the schema was changed a lot.

@chastichno based on how the schema versions work right now. It depends on data, if the data format is for schema 1 we show one form in mobile while schema 2 format shows a different form. @anildahalsussol has been working on it and I know a bit about it because I helped him. We will have a concrete answer in about 2 hours.

anildahalsussol commented 2 years ago

@adamdewey @sworup I have a question about how to use new version schemas.

Let's assume we had a patient survey schema with version "1". It was filled in for each patient, used during vaccinations etc. Then, we add a survey schema with version "2", which is different from version "1". What should happen now - should all the patient survey schemas become version 2? What if it's very different? The data would not be valid if the schema was changed a lot.

@chastichno @sworup : That was the question that we were having. What happens if version 1 schema doesn't match with schema 2 then should we make an adaptor to change the format of version 1 data to display in version 2 because whenever we get a new version that means it has some new field and new entry which they want for old doses as well. I worked as well to make an adaptor for V1 data to come to show in V2 but as per @sworup if we can make a more generic way then it can be easy in the future so I dropped that code. I think that what we need to do is make an adopter for showing old schema data to the new schema and save it as a new schema.

adamdewey commented 2 years ago

@anildahalsussol , @chastichno , @sworup My suggestion for this:

If a field in the V2 schema doesn't exist in the V1 schema:

If a field in the V1 schema doesn't exist in the V2 schema:

Not a valid scenario, we don't have any examples of this and don't expect to have it in the near future, so let's not worry about this scenario.

sworup commented 2 years ago
  • When editing V1 instance: Save using V2 schema

If a field in the V1 schema doesn't exist in the V2 schema:

Not a valid scenario, we don't have any examples of this and don't expect to have it in the near future, so let's not worry about this scenario.

@adamdewey so the new schema is guaranteed to have all the fields of older schemas in the exact hierarchical tree? You can only delete a field, but would not be able to move it without losing all the database values that previously used to go in that field.

@anildahalsussol if I remember correctly we talked about a live data scenario where field moved in the hierarchical tree. ???

adamdewey commented 2 years ago

Hey @sworup - at this stage yes, that's correct.

I don't think we need to make a general solution just now, just need to serve the Kiribati case. And in their case they just added a couple of fields, never deleted or moved anything.

sworup commented 2 years ago

@anildahalsussol @adamdewey @chastichno @ujwalSussol

We might have a case where this issue might resurface, once kiribati starts asking us to make extensive changes in schema, but for now the schema would not change too much just addition of fields. So this issue can be closed for now.