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 #3501 - Fix aggregation counts for elements removed and deleted #3543

Merged
merged 1 commit into from
Jul 10, 2023

Conversation

pijuszczyk
Copy link
Contributor

Two bugs were found and fixed:

  • previously elements removal and deletion were counted as two separate disappearances, leading to e.g. dropping below 0 in counters
  • removing comments did not affect post aggregations

The unit tests for aggregations could be greatly improved - just by accident I discovered that aggregations for comments counts per posts are outdated, in contrast to all the other aggregations. I don't have the time to handle it now but at least two more tests were added to verify the counting logic.

Fixes #3501.

…eleted

Two bugs were found and fixed:
- previously elements removal and deletion were counted as two separate disappearances
- removing comments did not affect post aggregations
Copy link
Member

@dessalines dessalines left a comment

Choose a reason for hiding this comment

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

Thanks for this! And great job on the unit tests too to verify it works.

end $$;

create or replace trigger post_aggregates_comment_count
after insert or delete or update of removed, deleted on comment
Copy link
Member

Choose a reason for hiding this comment

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

Thanks, this extra column check should speed things up a bit.

@dessalines dessalines merged commit 9c2490d into LemmyNet:main Jul 10, 2023
1 check passed
PawanHegde pushed a commit to PawanHegde/lemmy that referenced this pull request Jul 10, 2023
…eleted (LemmyNet#3543)

Two bugs were found and fixed:
- previously elements removal and deletion were counted as two separate disappearances
- removing comments did not affect post aggregations
Nutomic added a commit that referenced this pull request Jul 10, 2023
* Fix #3366: API does return plain HTML errors

* Fix Clippy errors

* Improve api response times by doing send_activity asynchronously (#3493)

* do send_activity after http response

* move to util function

* format

* fix prometheus

* make synchronous federation configurable

* cargo fmt

* empty

* empty

---------

Co-authored-by: Dessalines <dessalines@users.noreply.github.com>

* Updating `login.rs` with generic `incorrect_login` response. (#3549)

* Adding v0.18.1 and v0.18.0 release notes. (#3530)

* Update RELEASES.md (#3556)

added instruction to find the location of your docker directory (especially useful for those who used ansible since they never had to setup docker manually)

* Use async email sender (#3554)

* Upgrade all dependencies (#3526)

* Upgrade all dependencies

* as base64

* Adding phiresky to codeowners. (#3576)

* Error enum fixed (#3487)

* Create error type enum

* Replace magic string slices with LemmyErrorTypes

* Remove unused enum

* Add rename snake case to error enum

* Rename functions

* clippy

* Fix merge errors

* Serialize in PascalCase instead of snake_case

* Revert src/lib

* Add serialization tests

* Update translations

* Fix compilation error in test

* Fix another compilation error

* Add code for generating typescript types

* Various fixes to avoid breaking api

* impl From<LemmyErrorType> for LemmyError

* with_lemmy_type

* trigger ci

---------

Co-authored-by: SleeplessOne1917 <abias1122@gmail.com>

* Only update site_aggregates for local site (#3516)

* Fix #3501 - Fix aggregation counts for elements removed and deleted (#3543)

Two bugs were found and fixed:
- previously elements removal and deletion were counted as two separate disappearances
- removing comments did not affect post aggregations

* Use LemmyErrorType also make error_type compulsory

* Add missing import for jsonify_plain_text_errors

* Fix formatting

* Trying to make woodpecker run again

---------

Co-authored-by: phiresky <phireskyde+git@gmail.com>
Co-authored-by: Dessalines <dessalines@users.noreply.github.com>
Co-authored-by: rosenjcb <rosenjcb@gmail.com>
Co-authored-by: nixoye <12674582+nixoye@users.noreply.github.com>
Co-authored-by: dullbananas <dull.bananas0@gmail.com>
Co-authored-by: Nutomic <me@nutomic.com>
Co-authored-by: SleeplessOne1917 <abias1122@gmail.com>
Co-authored-by: Sander Saarend <sander@saarend.com>
Co-authored-by: Piotr Juszczyk <74842304+pijuszczyk@users.noreply.github.com>
Nutomic pushed a commit that referenced this pull request Jul 21, 2023
…3543)

Two bugs were found and fixed:
- previously elements removal and deletion were counted as two separate disappearances
- removing comments did not affect post aggregations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Posts with deleted comments can got into negative comment count
2 participants