Skip to content

IA-3772: Skip existing org units when importing data (hotfix)#2532

Merged
Phil-V merged 2 commits intomainfrom
IA-3772-orgunits-assertion-hotfix
Nov 6, 2025
Merged

IA-3772: Skip existing org units when importing data (hotfix)#2532
Phil-V merged 2 commits intomainfrom
IA-3772-orgunits-assertion-hotfix

Conversation

@Phil-V
Copy link
Copy Markdown
Contributor

@Phil-V Phil-V commented Nov 5, 2025

refs: IA-3772

Hotfix for exceptions raised by PR #2522

When attempting to create an OrgUnit that already exists, an AssertionError is thrown:
AssertionError: create() did not return an object instance.

This PR fixes the error and replicates the original behavior of ignoring the existing OrgUnits.

Self proofreading checklist

  • Did I use eslint and ruff formatters?
  • Is my code clear enough and well documented?
  • Are my typescript files well typed?
  • New translations have been added or updated if new strings have been introduced in the frontend
  • My migrations file are included
  • Are there enough tests?
  • Documentation has been included (for new feature)

Doc

Changes

  • Added tests that replicate the issue.
  • Fixed the issue by checking for existing OrgUnits and ignoring them.

How to test

  1. Testing the regular upload:
  • Create an org unit on mobile (note: you need to configure "sub org unit types to create" on the parent org unit type in the dashboard to allow the creation of org units on mobile).
  • Upload the new org unit
  • Find the payload in the api imports in the django admin.
  • Query the api manually with the same payload, it should be something like this:
curl --request POST \
  --url 'http://localhost:8081/api/mobile/orgunits/?app_id=hwbewoz&app_version=2600' \
  --header 'Authorization: Bearer <token>' \
  --header 'content-type: application/json' \
  --data '[
  {
    "id": "2740df90-5307-4bd9-aa5c-be9e6b144ffc",
    "name": "Test Areea Duplicated",
    "validation_status": "",
    "org_unit_type_id": "5",
    "parent_id": "4",
    "time": 0,
    "created_at": 1.762433961328E9,
    "updated_at": 1.762433961328E9
  }
]
  • The backend should return an empty response as the org unit is already saved and will be ignored.
  • You may try further queries with a mix of old and new UUIDs.
  1. Testing the bulk upload.
  • You need 2 project feature flags:
    • Entities > Mobile: Show entities screen
    • Data Validation > Mobile: Change requests
  • Create and upload an org unit with the mobile app
  • In the django admin, find the task associated with the upload and check that it's succesfull
  • Manually set its status to error
  • Restart the task in the dashboard
  • Check that it completes again without error.

You may want to test the features of the original PR #2522 as well

Print screen / video

/

Notes

Didn't want to change anything else, however, in removing the create() method on the serializer, I realized that both the code introduced by my PR and the original code set attributes on the OrgUnits that don't exist on the model and were silently ignored. I had to remove them when getting rid of the custom create logic as source and accuracy were passed to the OrgUnit's constructor and trigger TypeError.

Follow the Conventional Commits specification

The merge message of a pull request must follow the Conventional Commits specification.

This convention helps to automatically generate release notes.

Use lowercase for consistency.

Example:

fix: empty instance pop up

Refs: IA-3665

Note that the Jira reference is preceded by a line break.

Both the line break and the Jira reference are entered in the Add an optional extended description… field.

refs: IA-3772

- hotfix for exceptions raised by PR #2522
- add tests to replicate the issue
- remove serializer fields that don't exist on the model anymore
@Phil-V Phil-V marked this pull request as ready for review November 5, 2025 15:22
@Phil-V Phil-V requested a review from tdethier November 5, 2025 15:22
Copy link
Copy Markdown
Member

@tdethier tdethier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for diving a bit deeper on this 👍

@Phil-V Phil-V merged commit f993b63 into main Nov 6, 2025
3 checks passed
@Phil-V Phil-V deleted the IA-3772-orgunits-assertion-hotfix branch November 6, 2025 15:05
@Phil-V Phil-V restored the IA-3772-orgunits-assertion-hotfix branch November 6, 2025 15:30
@Phil-V Phil-V deleted the IA-3772-orgunits-assertion-hotfix branch November 6, 2025 15:52
@beygorghor beygorghor added hotfix Hot fix :fire: Released labels Nov 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hotfix Hot fix :fire: Released

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants