Allow to change the privacy level of external versions

[stsewd]

Only the field is added here,
the auth check is done in .com.

[stsewd]

Docs about privacy level would need to updated in new section added in #7823

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[ericholscher]

Looks like a good change with a few small nitpicks 👍

[stsewd]

I have changed this PR to include the querysets from .com, now that we have moved most of them here.

(This still needs one override in .com)

[ericholscher]

Looks like a good change, but just worried about performance and if we can contain this logic a bit more.

[ericholscher]

I'm a little worried about performance here -- we're adding a join and a few more queries on all our super common querysets. Can we not just change this on the external manager instead?

[stsewd]

Hmm, maybe, I'll need to take a look at where we are using those managers

readthedocs.org/readthedocs/builds/models.py

Lines 180 to 184 in ea574df

	objects = VersionManager.from_queryset(VersionQuerySet)()
	# Only include BRANCH, TAG, UNKNOWN type Versions.
	internal = InternalVersionManager.from_queryset(VersionQuerySet)()
	# Only include EXTERNAL type Versions.
	external = ExternalVersionManager.from_queryset(VersionQuerySet)()

[stsewd]

I think we can make this happen only when executing objects and external, as the internal manager already filters them out.

[stsewd]

I have used a partial class to pass a parameter indicating if the manager being used is for internal only versions.

[ericholscher]

@stsewd It seems like it might be cleaner to put this on a public method in the ExternalVersionManager instead of on the base VersionQueryset, since it only applies there (eg. we only want to run it on Version.external.public, I believe).

We should also do the same logic for the other querier (eg. on the ExternalBuildManager)

[stsewd]

We also want to include this on the default manager, since we do have calls to Version.objects.public/Build.objects.public.
And I'm not sure about moving these to the manager, these are more filters, and all of our querysets are there, we would nee to move all of those. I was about to use inheritance instead of the partial, but then we need to create three more classes (parent, InternalQuerysetBase, InternalQueryset(SeetingsOverride)) and one more override in .com :/

[ericholscher]

We don't need to worry about filtering external builds on Version.objects.public though right? They will already be filtered out, regardless of privacy level?

[stsewd]

I'm not sure if I understood what you mean 😅.

• Version.objects.public -> external and internal versions. Internal versions are checked against version.privacy_level, and external versions are checked against project.external_builds_privacy_level.
• Version.internal.public -> internal versions only, checked against version.privacy_level
• Version.external.public -> external versions only, checked against project.external_builds_privacy_level (for this case we could omit one filter from the current implementation, I'll do that)

But anyway, Version.objects.public need to check for both external and internal as that manager is expected to include both types of versions.

[ericholscher]

Ah yea, I was thinking Version.internal.public.

[ericholscher]

This looks good except for my question on the QS.

[ericholscher]

I think this works well -- hopefully it doesn't add much load to the DB.