PR#159: Add give_orphan endpoint - pagure-dist-git

pagure-dist-git

#159 Add give_orphan endpoint

Merged 2 years ago by humaton. Opened 2 years ago by lenkaseg.

lenkaseg/pagure-dist-git mattia into master

Add test for give_orphan: not in releng group

Lenka Segura • 2 years ago

42e9e7b

Add tests fro give_orphan endpoint

Lenka Segura • 2 years ago

94b6b10

Fix check in case of no user passed

Lenka Segura • 2 years ago

cf9c794

Add give_orphan endpoint

Mattia Verga • 2 years ago

e4d3f14

pagure_distgit/plugin.py

file modified

+105

		`@@ -387,6 +387,111 @@`
		`return flask.jsonify(output)`


		`+ @DISTGIT_NS.route("/give_orphan/<namespace>/<repo>", methods=["POST"])`
		`+ @api_login_required(acls=["modify_project"])`
		`+ @api_method`
		`+ def give_orphan_endpoint(namespace, repo):`
		`+ """`
		`+ Unretire/unorphan`
		`+ ------`
		`+ Updates the current point of contact of orphan packages.`
		`+`
		`+ ::`
		`+`
		`+ POST /_dg/give_orphan/<namespace>/<repo>`
		`+`
		`+ Input`
		`+ ^^^^^`
		`+`
		`+ +-----------------------+---------+--------------+------------------------+`
		`+ \| Key \| Type \| Optionality \| Description \|`
		`+ +=======================+=========+==============+========================+`
		+ \| ``user`` \| string \| Mandatory \| \| The user to set as \|
		`+ \| \| \| \| the new POC. \|`
		`+ +-----------------------+---------+--------------+------------------------+`
		`+`
		`+ Sample response`
		`+ ^^^^^^^^^^^^^^^`
		`+`
		`+ ::`
		`+`
		`+ {`
		`+ "point_of_contact": "mattia"`
		`+ }`
		`+ """`
		`+ _log.info("Received a request to unorphan: %s/%s", namespace, repo)`
		`+`
		`+ repo = _get_repo(repo, namespace=namespace)`
		`+ _check_token(repo, project_token=False)`
		`+`
		`+ user_authenticated = pagure.lib.query.get_user(`
		`+ flask.g.session, flask.g.fas_user.username`
		`+ )`
		`+ if not user_authenticated:`
		`+ raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOUSER)`
		`+`
		`+ user_grps = set(user_authenticated.groups)`
		`+ req_grps = set(["rel-eng", "admin"])`
		`+ if not user_grps.intersection(req_grps):`
		`+ raise pagure.exceptions.APIError(`
		`+ 403,`
		`+ error_code=APIERROR.ENOTHIGHENOUGH,`
		`+ errors="You must be in rel-eng or admin group to assign a package.",`
		`+ )`
		`+`
		`+ user=flask.request.values.get('user')`
		`+ if not user:`
		`+ raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOUSER,`
		`+ errors="You have to specify a user as point_of_contact.")`
		`+`
		`+ user_obj = pagure.lib.query.get_user(flask.g.session, user)`
		`+`
		`+ if repo.user.user != "orphan":`
		`+ raise pagure.exceptions.APIError(`
		`+ 401, error_code=APIERROR.EMODIFYPROJECTNOTALLOWED`
		`+ )`
		`+`
		`+ user_grps = set(user_obj.groups)`
		`+ req_grps = set(["packager"])`
		`+ if not user_grps.intersection(req_grps):`
		`+ raise pagure.exceptions.APIError(`
		`+ 403,`
		`+ error_code=APIERROR.ENOTHIGHENOUGH,`
		`+ errors="The new POC must be a packager to adopt a package.",`
		`+ )`
		`+`
		`+ # Check if the project is retired in PDC`
		`+ if not _is_active_in_pdc(repo.name, repo.namespace):`
		`+ raise pagure.exceptions.APIError(`
		`+ 400,`
		`+ error_code=APIERROR.EINVALIDREQ,`
		`+ errors="This project has been retired and cannot be unorphaned "`
		`+ "here, please unretire it.",`
		`+ )`
		`+`
		`+ try:`
		`+ repo.user = user_obj`
		`+ if repo.orphan_reason:`
		`+ reason = repo.orphan_reason`
		`+ flask.g.session.delete(reason)`
		`+ flask.g.session.add(repo)`
		`+ flask.g.session.commit()`
		`+ except SQLAlchemyError as err: # pragma: no cover`
		`+ flask.g.session.rollback()`
		`+ _log.exception(err)`
		`+ raise pagure.exceptions.APIError(400, error_code=APIERROR.EDBERROR)`
		`+`
		`+ pagure.lib.notify.log(`
		`+ repo,`
		`+ topic="project.adopt",`
		`+ msg=dict(project=repo.to_json(public=True), agent=user_obj.username),`
		`+ )`
		`+`
		`+ output = {"point_of_contact": repo.user.user}`
		`+`
		`+ return flask.jsonify(output)`
		`+`
		`+`
		`def query_oraculum_versions(package):`
		`"""`
		`Receives package versions from oraculum`

pagure_distgit_tests/test_plugin.py

file modified

+136

		`@@ -292,6 +292,16 @@`
		`True,`
		`[],`
		`)`
		`+ pagure.lib.query.add_group(`
		`+ self.session,`
		`+ "rel-eng",`
		`+ "rel-eng",`
		`+ "",`
		`+ "user",`
		`+ "pingou",`
		`+ True,`
		`+ [],`
		`+ )`

		`# Create a rpms/test4 project`
		`item = pagure.lib.model.Project(`
		`@@ -424,3 +434,129 @@`
		`"global_component=test4&name=rawhide&type=rpm",`
		`timeout=(30, 30),`
		`)`
		`+`
		`+ @patch("pagure_distgit.plugin._is_active_in_pdc")`
		`+ def test_give_orphan_no_user(self, mock_pdc):`
		`+ """Assert that the point_of_contact cannot be updated without specifying user."""`
		`+ mock_pdc.return_value = True`
		`+ headers = {"Authorization": "token aaabbbcccddd"}`
		`+ repo = pagure.lib.query.get_authorized_project(`
		`+ self.session,`
		`+ "test4",`
		`+ namespace="rpms",`
		`+ )`
		`+ output = self.app.post(`
		`+ "/_dg/give_orphan/rpms/test4",`
		`+ headers=headers,`
		`+ )`
		`+ assert output.status_code == 404`
		`+ data = json.loads(output.get_data(as_text=True))`
		`+ assert data["error"] == "No such user found"`
		`+ assert data["error_code"] == "ENOUSER"`
		`+ assert data["errors"] == "You have to specify a user as point_of_contact."`
		`+`
		`+ @patch("pagure_distgit.plugin._is_active_in_pdc")`
		`+ @patch("pagure_distgit.plugin.pagure.lib.notify.log")`
		`+ def test_give_orphan(self, mock_log, mock_pdc):`
		`+ """Assert that point of contact is correctly updated."""`
		`+ mock_pdc.return_value = True`
		`+ headers = {"Authorization": "token aaabbbcccddd"}`
		`+ orphan_user_obj = pagure.lib.query.get_user(self.session, "orphan")`
		`+ repo = pagure.lib.query.get_authorized_project(`
		`+ self.session,`
		`+ "test4",`
		`+ namespace="rpms",`
		`+ )`
		`+`
		`+ output = self.app.post(`
		`+ "/_dg/give_orphan/rpms/test4?user=pingou",`
		`+ headers=headers,`
		`+ )`
		`+ assert output.status_code == 200`
		`+ data = json.loads(output.get_data(as_text=True))`
		`+ self.assertDictEqual(data, {"point_of_contact": "pingou"})`
		`+`
		`+ @patch("pagure_distgit.plugin._is_active_in_pdc")`
		`+ @patch("pagure_distgit.plugin.pagure.lib.notify.log")`
		`+ def test_give_orphan_user_not_in_packager_group(self, mock_log, mock_pdc):`
		`+ """Assert that point of contact is correctly updated."""`
		`+ mock_pdc.return_value = True`
		`+ headers = {"Authorization": "token aaabbbcccddd"}`
		`+ orphan_user_obj = pagure.lib.query.get_user(self.session, "orphan")`
		`+ repo = pagure.lib.query.get_authorized_project(`
		`+ self.session,`
		`+ "test4",`
		`+ namespace="rpms",`
		`+ )`
		`+`
		`+ repo.user = orphan_user_obj`
		`+ self.session.add(repo)`
		`+ self.session.commit()`
		`+`
		`+ output = self.app.post(`
		`+ "/_dg/give_orphan/rpms/test4?user=orphan",`
		`+ headers=headers,`
		`+ )`
		`+ data = json.loads(output.get_data(as_text=True))`
		`+ assert (`
		`+ data["error"]`
		`+ == "You do not have sufficient permissions to perform this action"`
		`+ )`
		`+ assert data["error_code"] == "ENOTHIGHENOUGH"`
		`+ assert data["errors"] == "The new POC must be a packager to adopt a package."`
		`+`
		`+ @patch("pagure_distgit.plugin._is_active_in_pdc")`
		`+ @patch("pagure_distgit.plugin.pagure.lib.notify.log")`
		`+ def test_give_orphan_repo_user_not_orphan(self, mock_log, mock_pdc):`
		`+ """Assert that point of contact is correctly updated."""`
		`+ mock_pdc.return_value = True`
		`+ headers = {"Authorization": "token aaabbbcccddd"}`
		`+ orphan_user_obj = pagure.lib.query.get_user(self.session, "pingou")`
		`+ repo = pagure.lib.query.get_authorized_project(`
		`+ self.session,`
		`+ "test4",`
		`+ namespace="rpms",`
		`+ )`
		`+`
		`+ repo.user = orphan_user_obj`
		`+ self.session.add(repo)`
		`+ self.session.commit()`
		`+`
		`+ output = self.app.post(`
		`+ "/_dg/give_orphan/rpms/test4?user=orphan",`
		`+ headers=headers,`
		`+ )`
		`+ data = json.loads(output.get_data(as_text=True))`
		`+ assert data["error"] == "You are not allowed to modify this project"`
		`+ assert data["error_code"] == "EMODIFYPROJECTNOTALLOWED"`
		`+`
		`+ @patch("pagure_distgit.plugin._is_active_in_pdc")`
		`+ @patch("pagure_distgit.plugin.pagure.lib.notify.log")`
		`+ def test_give_orphan_user_not_in_releng_or_admin_group(self, mock_log, mock_pdc):`
		`+ """Assert that point of contact is correctly updated."""`
		`+ mock_pdc.return_value = True`
		`+ headers = {"Authorization": "token aaabbbcccddd"}`
		`+ repo = pagure.lib.query.get_authorized_project(`
		`+ self.session,`
		`+ "test4",`
		`+ namespace="rpms",`
		`+ )`
		`+ pagure.lib.query.delete_user_of_group(`
		`+ self.session,`
		`+ "pingou",`
		`+ "rel-eng",`
		`+ "pingou",`
		`+ False,`
		`+ force=True,`
		`+ )`
		`+ self.session.commit()`
		`+`
		`+ output = self.app.post(`
		`+ "/_dg/give_orphan/rpms/test4?user=foo",`
		`+ headers=headers,`
		`+ )`
		`+`
		`+ data = json.loads(output.get_data(as_text=True))`
		`+ assert output.status_code == 403`
		`+ assert data["errors"] == "You must be in rel-eng or admin group to assign a package."`
		`+ assert data["error_code"] == "ENOTHIGHENOUGH"`

lenkaseg commented 2 years ago

Signed-off-by: Mattia Verga mattia.verga@protonmail.com

lenkaseg commented 2 years ago

In process of writing tests, so currently work in progress.

lenkaseg commented 2 years ago

Thanks for the patch @mattia

Edited 2 years ago by lenkaseg

lenkaseg commented 2 years ago

Thinking about the patch, the function give_orphan_endpoint is very similar to take_orphan_endpoint, but the docstring is the same. Would it not be worthy to make the difference more apparent between those two?