Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

files_versions: add missing null check #44066

Merged
merged 1 commit into from
Mar 7, 2024
Merged

files_versions: add missing null check #44066

merged 1 commit into from
Mar 7, 2024

Conversation

pulsejet
Copy link
Member

@pulsejet pulsejet commented Mar 7, 2024

The fact that this can be null could indicate a deeper problem.

See pulsejet/memories#1053

@pulsejet pulsejet requested review from a team, ArtificialOwl, icewind1991 and nfebe and removed request for a team March 7, 2024 16:29
@szaimen szaimen added bug 3. to review Waiting for reviews labels Mar 7, 2024
@szaimen szaimen added this to the Nextcloud 29 milestone Mar 7, 2024
@susnux
Copy link
Contributor

susnux commented Mar 7, 2024

please rephrase your commit to match conventional commits, like:

fix(files_versions): Add missing null check

Copy link
Contributor

@emoral435 emoral435 left a comment

Choose a reason for hiding this comment

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

Nice change :) Currently looking at the files_version app right now, so if anything that can be the related to this pops up, I will come back here. but for now, this is a good change

Signed-off-by: Varun Patil <varunpatil@ucla.edu>
@pulsejet
Copy link
Member Author

pulsejet commented Mar 7, 2024

please rephrase your commit to match conventional commits, like:

fix(files_versions): Add missing null check

Done.

The fact that this can be null could indicate a deeper problem.

I just want to mention again that this might just be a band aid

@pulsejet pulsejet enabled auto-merge March 7, 2024 19:23
@pulsejet pulsejet merged commit 8a6ac51 into master Mar 7, 2024
160 checks passed
@pulsejet pulsejet deleted the pulsejet/ver-nc branch March 7, 2024 21:30
@artonge
Copy link
Contributor

artonge commented Mar 8, 2024

Band-aid indeed, reverting.
@pulsejet Please report whether #42891 could have solved the issue.
If it does not, then please provide more information to improve the path resolution logic.

@pulsejet
Copy link
Member Author

pulsejet commented Mar 8, 2024

@pulsejet Please report whether #42891 could have solved the issue.

I've no idea since I can't reproduce the bug anyway. But that seems like a reasonable fix.

@dinyar
Copy link

dinyar commented Apr 7, 2024

For the record: I have the same issue and could work around it by moving the image files from external storage to the "internal" one using the Nextcloud UI. I plan on moving them back to external storage again via the command line, but of course this isn't exactly practical.. (I'm on version 28.0.3 of Nexcloud and updated Memories yesterday.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants