Skip to content

Extract the file-field abstraction into the library - #33

Merged
domdinicola merged 2 commits into
developfrom
feature/320213-extract-file-field-abstraction
Oct 8, 2026
Merged

domdinicola merged 2 commits into
developfrom
feature/320213-extract-file-field-abstraction

Conversation

@roma-valor

Copy link
Copy Markdown
Contributor

AB#320213

Follow-up to the review on unicef/hope-country-workspace#466, which asked to
keep the offload itself in Country Workspace and promote only the generic
field abstraction here.

Closes #32

What moves into the library

  • FlexImageField / FlexImageInput plus the flex_image_widget template,
    now the canonical IMAGE field. The value is a flexfile:<uuid> reference,
    not the payload. clean() validates the upload and then returns the
    reference already on the record: only the consuming project holds both the
    uploaded file and the record to attach it to, so it is the only place that
    can store the payload and mint a new reference. A clean() that returned
    the file would put bytes into data meant to stay JSON.
  • hope_flex_fields.references, owning the reference scheme and nothing else:
    is_reference, is_data_uri, format_reference, parse_reference,
    flex_file_src, and the REFERENCE_PREFIX / DATA_URI_FORMAT /
    DATA_URI_PREFIX / DEFAULT_MIMETYPE constants. parse_reference returns
    None on a malformed value rather than raising, since these come from user
    data and from records written before the offload.
  • A FILE_URL_NAME config entry, reversed with the file id, so
    flex_file_src can build payload URLs without knowing a project's routes.
    CW sets it to workspace:flex_file. Unset or unreversible yields "" plus
    a warning, and the widget then renders the raw value as text instead of a
    broken image.
  • Base64ImageField / Base64ImageInput as deprecated shims, kept for one
    release. They are deliberately not registered: seeding a
    FieldDefinition for a deprecated type on every fresh install would offer
    it to users who should not pick it, so a project still migrating away
    registers it itself (CW already does this in its reverse script). The
    deprecated widget inherits the new rendering, which handles both references
    and legacy data: URIs, so swapping the field type and migrating the stored
    data can land in either order.

The discovery API asked for in #32 (field.is_file,
DataChecker.get_file_field_names(with_prefix=...)) already shipped in 0.8.4
and 0.9; this PR documents it rather than changing it.

What deliberately stays with the consumer

Storage and access control: the FlexFieldFile model, the storage helpers,
the tenant/program-scoped view that serves payloads, and the data migration.
Those depend on CW's Validable, pghistory and RDP flows, and are out of
scope per the review.

Migration note, worth a look

0018_add_fleximagefield seeds the FieldDefinition by repointing any row
already under that name, rather than calling create_default_fields.
FieldDefinition.name is unique, and CW already has a FlexImageField row
pointing at its own class, so a blind get_or_create(name=..., field_type=...)
would fail the unique constraint on upgrade. The update also goes through the
queryset so the old field_type is never deserialized, which would raise as
soon as CW drops its class. This keeps the CW-side change to deleting its
local classes and importing from here.

Decisions I would like confirmed

  • Pillow is in the dev group only, needed to exercise the upload path. The
    library now registers an image field that any install can select, and
    forms.ImageField.to_python does an unguarded from PIL import Image, so a
    consumer without Pillow gets an ImportError on upload. Django has the same
    gap, hence the conservative choice, and CW already declares pillow>=12.3.
    Happy to promote it to a runtime dependency or an extra instead.
  • get_file_field_names is unchanged. Extract file-field abstraction from CW PR 466 #32 describes CW's union wrapper as a
    workaround, but the with_prefix parameter already covers both forms, so
    CW's remaining two-liner is a convenience for call sites keying on either
    name. Say the word if you want a single call returning the union.

Testing

205 tests pass, up from 148. references.py, fields.py and widgets.py are
at 100% line and branch coverage. New tests cover malformed references, legacy
data: pass-through, None/empty handling, the unset and unreversible route
cases, upload/clear/no-change paths through clean(), widget rendering for
reference, legacy and unresolvable values, and the migration repointing a
definition a project already owns without duplicating it.

tests/test_api.py moves its object count 34 -> 35, as it did when
IdentityField was added in #25, since one more field type is registered by
default.

Promotes the generic parts of the flex-file offload from Country
Workspace so that consumers share one field type and one reference
format instead of each redefining them.
@codecov

codecov Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.83%. Comparing base (c144b3d) to head (d85b419).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop      #33      +/-   ##
===========================================
+ Coverage    95.62%   95.83%   +0.20%     
===========================================
  Files           28       29       +1     
  Lines         1372     1441      +69     
  Branches       149      157       +8     
===========================================
+ Hits          1312     1381      +69     
  Misses          40       40              
  Partials        20       20              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@roma-valor
roma-valor marked this pull request as ready for review October 2, 2026 09:09
Comment thread pyproject.toml Outdated
Comment thread src/hope_flex_fields/migrations/0018_add_fleximagefield.py Outdated
@roma-valor
roma-valor requested a review from saxix October 6, 2026 14:47
@domdinicola
domdinicola merged commit 74aa661 into develop Oct 8, 2026
9 checks passed
@domdinicola
domdinicola deleted the feature/320213-extract-file-field-abstraction branch October 8, 2026 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extract file-field abstraction from CW PR 466

4 participants