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

fix: skip exporting a deleted card #6692

Merged
merged 1 commit into from
Feb 4, 2025
Merged

fix: skip exporting a deleted card #6692

merged 1 commit into from
Feb 4, 2025

Conversation

elzody
Copy link
Contributor

@elzody elzody commented Jan 24, 2025

Summary

Checks if the card is deleted before continuing -- if it is, just move on

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Tests (unit, integration, api and/or acceptance) are included
  • Documentation (manuals or wiki) has been updated or is not required

Copy link
Contributor

🐢 Performance warning.
It looks like the query count of the integration tests increased with this PR.
Database query count is now 71755 was 71221 (+0.74%)
Please check your code again. If you added a new test this can be expected and the base value in tests/integration/base-query-count.txt can be increased.

@elzody elzody marked this pull request as ready for review January 24, 2025 21:04
Copy link
Contributor

🐢 Performance warning.
It looks like the query count of the integration tests increased with this PR.
Database query count is now 71755 was 71221 (+0.74%)
Please check your code again. If you added a new test this can be expected and the base value in tests/integration/base-query-count.txt can be increased.

@elzody
Copy link
Contributor Author

elzody commented Jan 27, 2025

@luka-nextcloud @grnd-alt I never touched any of the integration tests, so I'm not sure why the query count would have increased. The only tests I added were unit tests, which I would assume would also increase the query count since the command makes database queries. Are unit tests included in the query diff? The message is a bit confusing.

@elzody
Copy link
Contributor Author

elzody commented Feb 3, 2025

@grnd-alt I cannot figure out why it says the number of database queries has increased. If anything, the changes should make it query the database less. And the unit tests, from what I can see, should not be affecting it. Am I missing something?

Maybe @luka-nextcloud can have a look?

Signed-off-by: Elizabeth Danzberger <[email protected]>
Copy link
Contributor

github-actions bot commented Feb 4, 2025

🐢 Performance warning.
It looks like the query count of the integration tests increased with this PR.
Database query count is now 71779 was 71221 (+0.78%)
Please check your code again. If you added a new test this can be expected and the base value in tests/integration/base-query-count.txt can be increased.

@elzody
Copy link
Contributor Author

elzody commented Feb 4, 2025

/backport to stable31

@elzody
Copy link
Contributor Author

elzody commented Feb 4, 2025

/backport to stable30

@elzody
Copy link
Contributor Author

elzody commented Feb 4, 2025

/backport to stable29

@elzody elzody merged commit 0780b10 into main Feb 4, 2025
37 checks passed
@elzody elzody deleted the fix/6446 branch February 4, 2025 18:27
@luka-nextcloud
Copy link
Contributor

@grnd-alt I cannot figure out why it says the number of database queries has increased. If anything, the changes should make it query the database less. And the unit tests, from what I can see, should not be affecting it. Am I missing something?

Maybe @luka-nextcloud can have a look?

@elzody It is the query count total, not slow query. I guess it might be related to the update of server repository. It's just a few queries of increasing. We just need to update the tests/integration/base-query-count.txt and nothing to worry about.

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.

Deleted cards break "occ deck:export"
2 participants