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

Show "pending" activity in devhub for all versions and not just the latest one #22681

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

diox
Copy link
Member

@diox diox commented Sep 18, 2024

Fixes: mozilla/addons#15024

Context

See the issue - without this change, for a given add-on in devhub, only the last version in each channel shows the "pending" activity count. This makes it shown on all versions, while not increasing the query count (plenty more to optimize there though, but out of scope)

Testing

See the issue but also the added tests of the model method + test around the view that were added.

@diox diox changed the title Devhub show pending count for all versions Show "pending" activity in devhub for all versions and not just the latest one Sep 18, 2024
@diox diox marked this pull request as ready for review September 19, 2024 15:52
@diox diox requested review from a team and KevinMind and removed request for a team September 19, 2024 15:55
@@ -99,6 +99,129 @@ def test_rejected_version_still_valid(self):
assert self.token.is_valid()


class TestActivityLogManager(TestCase):
def test_pending_for_developer(self):
to_create = (
Copy link
Contributor

Choose a reason for hiding this comment

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

curious why not using a parameterized test here? Again seems to predate but this would parallelize the tests and let each fail independently of the others.

Copy link
Member Author

Choose a reason for hiding this comment

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

@pytest.mark.parametrize is not really usable with TestCase - it wants to feed all the arguments to the function, so that doesn't play well with methods that should receive self as the first argument.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right. In those cases I've just used functional tests. Just wondering as I've used it before and wonder if we have a stigma against it or not.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't love @parametrize because of that limitation (and general awkwardness around how arguments are defined when using it) but nobody would stop you if you were to use it in new code. We do have several already.

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.

[Task]: Show "pending" activity in devhub for all versions and not just the latest one
3 participants