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

QuerySets: remove unused overrides #8299

Merged
merged 3 commits into from Sep 1, 2021
Merged

QuerySets: remove unused overrides #8299

merged 3 commits into from Sep 1, 2021

Conversation

stsewd
Copy link
Member

@stsewd stsewd commented Jun 29, 2021

We don't user these in .com, this is on top of #8298

With the aim to bring logic from .com to .org more closer
I'm moving the organization's filters to .org.
They are going to be used if `RTD_ALLOW_ORGANIZATIONS` is True,
but they eventually will be merged with the current querysets
when organizations and normal projects are supported in .org.

In .com we are relying on user.projects having the
the projects where the user is member of and organization owner
(we are syncing this with signals).
We don't rely on that hack anymore and always check from the
organization models. The signals won't be removed for now,
but shouldn't be needed anymore.

To re-use more code I have brought the SSO concept here,
but isn't implemented yet, we can bring the SSO models and logic later
easily.

This could be seen as more queries,
but the main ones were already being executed in .com.
The other ones are executed only when using the API V2,
so I don't think that would be a problem.

I started this refactor using a mixin,
but then we would need to override every single
queryset in .com, using composition only requires
one override (already used),
and doesn't bring other methods into the class.
@@ -179,7 +179,6 @@ def get_queryset(self):
queryset = super().get_queryset()
queryset = (
queryset
.internal()
Copy link
Member Author

Choose a reason for hiding this comment

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

We are doing filtering per-version in

queryset = (
doc_obj.get_queryset()
.filter(project=version.project, version=version, build=build)
)
, and we don't trigger the index task for external versions, so this query doesn't change anything.

We don't user these in .com
@stsewd stsewd requested a review from a team June 30, 2021 00:18
Copy link
Member

@humitos humitos left a comment

Choose a reason for hiding this comment

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

Looks good! Let's merge this after we finish our conversation in #8298 to avoid mixing it with unrelated changes.

Base automatically changed from take-orgs-into-consideration to master September 1, 2021 21:50
@stsewd stsewd merged commit 1de8db7 into master Sep 1, 2021
@stsewd stsewd deleted the remove-unused-overrides branch September 1, 2021 22:14
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.

None yet

2 participants