PR#83: dist_git_auth: fix pr_only check logic - pagure-dist-git

pagure-dist-git

#83 dist_git_auth: fix pr_only check logic

Merged 5 years ago by pingou. Opened 5 years ago by jlanda.

jlanda/pagure-dist-git fix-pr-only into master

tests: test project level pull requests only

Julen Landa Alustiza • 5 years ago

7304c5c

dist_git_auth: fix pr_only check logic

Julen Landa Alustiza • 5 years ago

55cb515

dist_git_auth.py

file modified

+2 -2

		`@@ -254,8 +254,8 @@`
		`# this is not a fork, only allow pushing if this is a PR merge.`
		`pr_only = project.settings.get("pull_request_access_only", False)`
		`if (`
		`- pr_only`
		`- or (self.global_pr_only and not project.is_fork)`
		`+ (pr_only`
		`+ or (self.global_pr_only and not project.is_fork))`
		`and pull_request is None`
		`):`
		`self.info("A pull request is required for this branch")`

dist_git_auth_tests.py

file modified

+46 -1

		`@@ -117,7 +117,7 @@`
		`self.dga = None`
		`super(DistGitAuthTests, self).tearDown()`

		`- def create_namespaced_project(self, namespace, name, is_fork=False):`
		`+ def create_namespaced_project(self, namespace, name, is_fork=False, settings={}):`
		`item = pagure.lib.model.Project(`
		`user_id=1, # pingou`
		`name=name,`
		`@@ -126,6 +126,7 @@`
		`description='namespaced test project',`
		`hook_token='aaabbbeee',`
		`namespace=namespace,`
		`+ settings=settings`
		`)`
		`item.close_status = [`
		`'Invalid', 'Insufficient data', 'Fixed', 'Duplicate']`
		`@@ -371,7 +372,51 @@`
		`)`

		`self.expect_info_msg("Committer push to unprotected")`
		`+`
		`+ def test_unprotected_pr_required_repo_pr_only_no_pr(self):`
		`+ settings = {"pull_request_access_only": True}`
		`+ project = self.create_namespaced_project('unprotected', 'test', settings=settings)`

		`+ self.assertFalse(`
		`+ self.dga.check_acl(`
		`+ self.session,`
		`+ project=project,`
		`+ username="pingou",`
		`+ refname="refs/heads/mywip",`
		`+ pull_request=None,`
		`+ repodir=None,`
		`+ repotype='main',`
		`+ revfrom=None,`
		`+ revto=None,`
		`+ is_internal=False,`
		`+ )`
		`+ )`
		`+`
		`+ self.expect_info_msg("A pull request is required for this branch")`
		`+`
		`+ def test_unprotected_pr_required_repo_pr_only(self):`
		`+ settings = {"pull_request_access_only": True}`
		`+ project = self.create_namespaced_project(`
		`+ 'unprotected',`
		`+ 'test',`
		`+ settings=settings`
		`+ )`
		`+ self.assertTrue(`
		`+ self.dga.check_acl(`
		`+ self.session,`
		`+ project=project,`
		`+ username="pingou",`
		`+ refname="refs/heads/mywip",`
		`+ pull_request="pull_request",`
		`+ repodir=None,`
		`+ repotype='main',`
		`+ revfrom=None,`
		`+ revto=None,`
		`+ is_internal=False,`
		`+ )`
		`+ )`
		`+`
		`+ self.expect_info_msg("Committer push to unprotected")`

		`class DistGitAuthTestsFedora(DistGitAuthTests):`
		`dga_config = {`

jlanda commented 5 years ago

This fixes #84

rebased onto 55cb515

5 years ago

2 new commits added

tests: test project level pull requests only
dist_git_auth: fix pr_only check logic

5 years ago

pingou commented 5 years ago

I'll need to run the tests locally before merging this but from a quick looks good.

I wonder if we could work something in the indentation to make the logical block clearer (indent within the parenthesis?), I'll likely play around with this when testing it :)

pingou commented 5 years ago

Ok tests are passing locally and the change looks fine, so let's get this in.
I'll open a new PR running black on the code :)