fix(mobile): add missing backward-compat defaults in openapi_patching

[timonrieger]

Description

The

• prerelease field was added to ServerVersionResponseDto in feat: release candidate support #28665
• createdAt was added to SyncAssetV1/SyncAssetV2 in feat: recently added assets page #28272
• AssetEditActionItemDtoParameters has always had a union type mismatch since feat: image editing #24155

All three had their assertions patched out in the dart client, hiding the bugs. The regenerated dart client (from #27231) will (on merge) restore those assertions, causing debug-mode crashes.

Fixes # (issue)

How Has This Been Tested?

• Test A
• Test B

Screenshots (if appropriate)

Checklist:

• I have carefully read CONTRIBUTING.md
• I have performed a self-review of my own code
• I have made corresponding changes to the documentation if applicable
• I have no unrelated changes in the PR.
• I have confirmed that any new dependencies are strictly necessary.
• I have written tests for new code (if applicable)
• I have followed naming conventions/patterns in the surrounding code
• All code in src/services/ uses repositories implementations for database calls, filesystem operations, etc.
• All code in src/repositories/ is pretty basic/simple and does not have any immich specific logic (that belongs in src/services/)

Please describe to which degree, if any, an LLM was used in creating this pull request.

...

[jrasm91]

I'm surprised this is needed, since it's nullable.

[timonrieger]

but not optional 😁

[timonrieger]

it does not raise on main atm, due to this patch stripping assertions. While testing #27231, this raised tho in the mobile app

Logs

'package:openapi/model/server_version_response_dto.dart': Failed assertion: line 95 pos 16: 'json.containsKey(r'prerelease')': Required key "ServerVersionResponseDto[prerelease]" is missing from JSON.

[timonrieger]

so effectively, prerelease is still required (see openapi spec) and thus needs this…unless we mark the field as optional, too

[jrasm91]

We might need to fix other issues when we make that change. There are other required, nullable properties.

[jrasm91]

Yeah so these aren't strictly required. Nullable values will fallback to null by default. Idk if need/want to add back in the required assertions.

[shenlong-tanwen]

@timonrieger This might be an issue whenever a server promotes some field to required and the mobile app has to patch it. Rather than tracking this, maybe we should drop the assertions

[timonrieger]

i disagree. In debug/dev these assertions catch bugs which otherwise stay silent and could cascade further down if the response was used. i don't see why we would want to ignore these bugs, even if they don't affect us at the moment 🤔

[timonrieger]

i mean it's basically response validation on the client-side (which we discussed server-side alr btw. #27901 😄 ). in prod build, these assertions get stripped anyways.

[jrasm91]

I personally don't find any value in assertions for required null values. The only thing it actually does it make the mobile App break in development when testing against an old server.

[immich-push-o-matic]

📱 Android release APK (universal) — d2ba1061f2dd33054512d3e99b9298b3186da407

Download: https://github.com/immich-app/immich/actions/runs/26786940279/artifacts/7345065820

QR code

Installs as a separate app (applicationId app.alextran.immich.pr28755), so it coexists with the Play Store version and any other PR builds.

[shenlong-tanwen]

@timonrieger Do we still need this now that we removed the debug assertions?

[timonrieger]

Do we still need this now that we removed the debug assertions?

yes. i likely was not clear enough on the other PR, but I meant to say that I removed the patch that removed these assertions 😅 (clarified in #27231 (comment)). In the end it's up to you which path we go. I made my point above. If we keep these assertions removed (as they are now), we necessarily don't "need" this change here. As said, those are correctness fixes. Let me know what you prefer :)

[jrasm91]

I would prefer keeping the patch that removes the assertions and not needing to add these type of patches.

[timonrieger]

as per team vote, we'll continue to suppress assertions. Thus closing this.