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

Cleanup versions entities in versions:clean command #43263

Merged
merged 1 commit into from
Mar 6, 2024

Conversation

artonge
Copy link
Contributor

@artonge artonge commented Feb 1, 2024

Forgoten place where we need to remove versions entities in the database.

Fix #39046

@artonge artonge changed the title Merge pull request #43203 from nextcloud/fix/cleanup-register-command-with-di Cleanup versions entity in during versions:clean command Feb 1, 2024
@artonge artonge changed the title Cleanup versions entity in during versions:clean command Cleanup versions entities in during versions:clean command Feb 1, 2024
@artonge artonge self-assigned this Feb 1, 2024
@artonge artonge added bug 3. to review Waiting for reviews feature: versions php Pull requests that update Php code labels Feb 1, 2024
@artonge artonge added this to the Nextcloud 29 milestone Feb 1, 2024
@artonge
Copy link
Contributor Author

artonge commented Feb 1, 2024

/backport to stable28

@artonge
Copy link
Contributor Author

artonge commented Feb 1, 2024

/backport to stable27

@artonge
Copy link
Contributor Author

artonge commented Feb 1, 2024

/backport to stable26

@artonge artonge force-pushed the artonge/fix/versions_cleanup branch 2 times, most recently from 9df8844 to b2cc661 Compare February 1, 2024 16:51
@artonge

This comment was marked as resolved.

@nickvergessen
Copy link
Member

  1. OCA\Files_Versions\Tests\Command\CleanupTest::testDeleteVersions with data set #0 (true)
    ArgumentCountError: Too few arguments to function OCA\Files_Versions\Command\CleanUp::__construct(), 2 passed in /home/runner/work/server/server/apps/files_versions/tests/Command/CleanupTest.php on line 60 and exactly 3 expected

@artonge artonge force-pushed the artonge/fix/versions_cleanup branch from 0b2b1a9 to b0887c0 Compare March 4, 2024 14:51
@artonge artonge force-pushed the artonge/fix/versions_cleanup branch 2 times, most recently from ca4d80b to 94fdd1c Compare March 4, 2024 15:05
@artonge artonge force-pushed the artonge/fix/versions_cleanup branch 5 times, most recently from 11a31d5 to 915c02b Compare March 4, 2024 17:17
@artonge artonge force-pushed the artonge/fix/versions_cleanup branch from 915c02b to ea4898d Compare March 5, 2024 10:39
@artonge artonge force-pushed the artonge/fix/versions_cleanup branch 2 times, most recently from ad0d158 to aead675 Compare March 5, 2024 13:56
Copy link
Member

@icewind1991 icewind1991 left a comment

Choose a reason for hiding this comment

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

For bonus phpoints™ you can split off the fileid chunk fetching into a generator to hopefully make it bit more readable than the mixing of queries

@artonge artonge force-pushed the artonge/fix/versions_cleanup branch from aead675 to ed3f4ed Compare March 5, 2024 15:15
@artonge
Copy link
Contributor Author

artonge commented Mar 5, 2024

For bonus phpoints™ you can split off the fileid chunk fetching into a generator to hopefully make it bit more readable than the mixing of queries

Indeed, improves separation of concern, thanks for the tip :)

@artonge artonge dismissed nickvergessen’s stale review March 5, 2024 15:35

Robin reviewed the query :)

@icewind1991
Copy link
Member

icewind1991 commented Mar 5, 2024

✨ +1 phpoint™ for Louis ✨

Signed-off-by: Louis Chemineau <louis@chmn.me>
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 feature: versions php Pull requests that update Php code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Versions still appear in the Files view after running php occ files_versions:cleanup
5 participants