Skip to content

feat: view the user's app version on the User Management page#20306

Closed
aviv926 wants to merge 13 commits intoimmich-app:mainfrom
aviv926:app-version
Closed

feat: view the user's app version on the User Management page#20306
aviv926 wants to merge 13 commits intoimmich-app:mainfrom
aviv926:app-version

Conversation

@aviv926
Copy link
Collaborator

@aviv926 aviv926 commented Jul 27, 2025

Description

This PR adds information for the system administrator about which version each user is currently using. This is updated every time the user accesses one of the Android or iPhone mobile applications only by information coming from the User-Agent header.

This change also adds a new database column called appVersion to store the information for each user.

How Has This Been Tested?

I tried accessing the app for Android and the app for iPhone and saw that the version is updated on the User Management page even if a version update is made to the app and even if the user accesses it.
To ensure that the latest version is always displayed for each user, it updates the App version every time a request is sent to the server by the user from one of the mobile applications.

I tried accessing from the browser and making sure that it does not affect the App version on the User Management page
Although it is not possible to access with port 2283 as we access from the app (I am not sure if this makes any difference at all?), I was able to connect with port 3000 as we access on the computer and verify that the App version did not change as expected.

Full disclosure: I'm not good with typescript but immich was an inspiration for me to start learning TS so I hope what's here is good enough.
Yes I had to use ML to solve some problems along the way and work with regex and kysely so I hope it's not too bad.
I tried to do all the testing to make sure it would be good enough :)

Screenshots (if appropriate)

image

Checklist:

  • 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 (I hope so.)
  • 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/)

@aviv926 aviv926 marked this pull request as ready for review July 27, 2025 01:42
@aviv926 aviv926 requested a review from danieldietzler as a code owner July 27, 2025 01:42
Copy link
Member

@danieldietzler danieldietzler left a comment

Choose a reason for hiding this comment

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

Seems fine to me, thanks!

Copy link
Member

Choose a reason for hiding this comment

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

Remove this. src/migrations are the old migrations, all migrations are now in schema/migrations

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe we should update the documentation regarding this as I used the documentation to understand how to work with migrations

https://immich.app/docs/developer/database-migrations/

Copy link
Member

Choose a reason for hiding this comment

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

  1. Move the migration file to folder ./server/src/schema/migrations in your code editor.

Isn't that correct? 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, but that doesn't explain deleting the original file in server/src/schema after npm run migrations
Or am I just not understanding it correctly 🙇🏻‍♂️

@UpdateIdColumn({ index: true })
updateId!: Generated<string>;

@Column({ type: 'character varying', nullable: true, default: null })
Copy link
Member

Choose a reason for hiding this comment

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

I don't think you need a default if it's null anyways?

Copy link
Collaborator Author

@aviv926 aviv926 Jul 27, 2025

Choose a reason for hiding this comment

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

I wasn't sure, so I used the logic as we use in

  @Column({ type: 'character varying', nullable: true, default: null })
  color!: string | null;

If you think it's not necessary (as it seems I will remove it from appVersion Column)

@jrasm91
Copy link
Member

jrasm91 commented Jul 27, 2025

Surely this should be per session not per user? Also this could probably be called "Client version".

@danieldietzler
Copy link
Member

Surely this should be per session not per user? Also this could probably be called "Client version".

Yeah we already talked about this elsewhere too and decided on per session.

@bo0tzz bo0tzz removed the preview label Jul 31, 2025
@aviv926 aviv926 added preview and removed preview labels Aug 7, 2025
@aviv926
Copy link
Collaborator Author

aviv926 commented Aug 24, 2025

@alextran1502
Hey Alex, have you considered merge this PR?
I have seen that you add preview label to this PR but for some reason it's still don't work because of error with the github workflow.

@danieldietzler
Copy link
Member

The preview label does not work for forks. I don't think you've changed this to be per-session yet, have you? That'd be a requirement as that's the only way this makes sense.

@aviv926
Copy link
Collaborator Author

aviv926 commented Aug 27, 2025

Closed. New PR in with the required changes in #21345

@aviv926 aviv926 closed this Aug 27, 2025
@bo0tzz bo0tzz removed the preview label Jan 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants