Add file to CDE #4910
Add file to CDE #4910
Conversation
| else | ||
| submitted = Array.wrap(value) |
There was a problem hiding this comment.
Currently, when any CDE value changes, we delete and replace all of them. See regression test farther down on this PR. I think this is a bug, but until now it wouldn't have had any impact except that such CDEs would have incorrect creation timestamps. The reason to fix it now is to prevent deletion and re-creation of File records.
There was a problem hiding this comment.
Ahh yep, thank you for addressing that!
How will it handle situations where there are repeated values that exist or are submitted? Like [{value_string: "foo"}, {value_string: "foo"}] (I'm thinking on an open-choice field, perhaps).
I don't think it's really necessary to preserve both values if they are identical, but it shouldn't explode or do something weird- maybe worth having a test case.
There was a problem hiding this comment.
I added a test case. It doesn't explode but it does do something weird - see comment below https://github.com/greenriver/hmis-warehouse/pull/4910/files#r1876005116
Co-authored-by: Gig <gig@greenriver.org>
| ) | ||
| process_record(record: record, hud_values: hud_values, user: hmis_user, definition: definition) | ||
|
|
||
| # expect(record.custom_data_elements.size).to eq(4) |
There was a problem hiding this comment.
The commented-out line fails, since custom_data_elements.size is equal to 3. The last foo value doesn't get properly added because it's considered an existing value.
I think the most graceful way to deal with this is to just prevent submission of duplicate values. That way we don't have to keep track of value counters or anything like that. Maybe in normalize_custom_field_value?
There was a problem hiding this comment.
I implemented this, let me know what you think
| @@ -229,7 +229,8 @@ def custom_case_notes(...) | |||
| end | |||
|
|
|||
| def files(**args) | |||
| resolve_files(**args) | |||
| # Exclude files that have been uploaded by custom assessments | |||
| resolve_files(object.files.left_joins(:custom_data_element).merge(Hmis::Hud::CustomDataElement.where(id: nil)), **args) | |||
There was a problem hiding this comment.
This is equivalent to .where.missing(:custom_data_element)! learned that one recently, I think it's neat
> Hmis::File.where.missing(:custom_data_element).count
Hmis::File Count (3.0ms) SELECT COUNT(*) FROM "files" LEFT OUTER JOIN "CustomDataElements" "custom_data_element" ON "custom_data_element"."DateDeleted" IS NULL AND "custom_data_element"."value_file_id" = "files"."id" WHERE "files"."type" = $1 AND "files"."deleted_at" IS NULL AND "custom_data_element"."id" IS NULL [["type", "Hmis::File"]]
> Hmis::File.left_joins(:custom_data_element).merge(Hmis::Hud::CustomDataElement.where(id: nil)).count
Hmis::File Count (3.3ms) SELECT COUNT(*) FROM "files" LEFT OUTER JOIN "CustomDataElements" ON "CustomDataElements"."DateDeleted" IS NULL AND "CustomDataElements"."value_file_id" = "files"."id" WHERE "files"."type" = $1 AND "files"."deleted_at" IS NULL AND "CustomDataElements"."DateDeleted" IS NULL AND "CustomDataElements"."id" IS NULL [["type", "Hmis::File"]]
Merging this PR
Description
GH issue: https://github.com/open-path/Green-River/issues/6208
This PR
value_file_id, alongside the existingvalue_string,value_json, etc.To test:
Type of change
New feature
Checklist before requesting review