Feature/improve date selection #9160
Conversation
| const asset = await this.assetRepository.getById(id); | ||
| const oldCreatedAtDate = asset?.fileCreatedAt && DateTime.fromJSDate(asset.fileCreatedAt); | ||
| let newDateTimeString = dateTimeOriginal; | ||
| if (dateTimeOriginal && keepTimeUnchanged && oldCreatedAtDate) { | ||
| let newDateTime = DateTime.fromISO(dateTimeOriginal); | ||
|
|
||
| newDateTime = newDateTime.set({ | ||
| hour: oldCreatedAtDate.hour, | ||
| minute: oldCreatedAtDate?.minute, | ||
| second: oldCreatedAtDate.second, | ||
| }); | ||
| const newDateTimeStringWithNull = newDateTime?.toISO(); | ||
| newDateTimeString = newDateTimeStringWithNull === null ? undefined : newDateTimeStringWithNull; | ||
| } |
There was a problem hiding this comment.
This is inside a for loop, which might be called with 10k assets. Loading an asset from the database one at a time is going to be a problem. In hindsight, calling updateMetadata one at a time, which eventually calls upsertExif might not be the best either. I would probably recommend finding a more optimized solution. Ideally we move this logic into updateMetadata itself, and change that method to accept a list of ids to update. Only if the keepTimeUnchanged property is set, we can use the assetRepo.getByIds methods to load them in a single query.
There was a problem hiding this comment.
We could also move the "keepTimeUnchanged" logic inside the SIDECAR_WRITE job or create a new one for that case.
The problem I see with changing multiple assets in one Job is the error handling.
What should happen if one of the metadata updates fail?
And maybe one side question:
is await this.jobRepository.queue waiting for the job to complete, or is it fire and forget. And what would happen if the SIDECAR_WRITE fails and the
await this.assetRepository.updateAll(ids, options); // asset.services.ts L 343
still is done.
There was a problem hiding this comment.
.queue is waiting for the job to be added to the queue. It is fine if the job takes longer to execute since it happens in the background.
Currently the bulk update (and single update) method optimistically update the database so the UI can refresh immediately and see the changes, but then persist the changes to a sidecar file so future metadata extraction doesn't erase/override the changes.
It is important that the initial implementation is fast, since the caller is waiting. Updating every asset individually makes that a little bit more difficult, especially if the asset needs to be read ahead of time.
There was a problem hiding this comment.
The options are to not do any update immediately, but defer it to the job. The user might be confused as the time isn't updated and the assets still appear in the wrong place for some time.
Or, update each asset one at a time, but optimize the loading to be in a single database request.
In the change date dialog the inital date is not now anymore but the date of the first selected asset
In the change date dialog there is now an option to ignore time change and set the original time of the asset
Co-authored-by: Daniel Dietzler <36593685+danieldietzler@users.noreply.github.com>
sebbi08
force-pushed
the
feature/improve_date_selection
branch
from
6dd0a73
to
c6fc22b
Compare
This PR contains two changes, the smaller one sets the initial date of the date change dialog to the date of the first selected asset.
The bigger one adds an option to ignore the Time change and sets the original time.
The idea behind this PR is that I often upload images/videos from my GoPro or Drone. But both of them have sometimes a problem with the date and or the time.
So if I bulk change all the asses from one day, where date and time are off, I want to keep the "false" time because this will keep the assets in the right order. The smaller change is pure laziness, as I think it is easier in the date picket to get to "Today" than it is to go back "3 years" to the original asset date. I picked the first asset because I think most users will use the change date on similar dated assets.