pagure

Clone

#3320 Add a new API endpoint allowing to update watch status on a project
Merged 5 years ago by pingou. Opened 5 years ago by pingou.
api_update_watch  into  master

file modified
+3 
@@ -464,6 +464,8 @@ 
 

      api_new_branch_doc = load_doc(project.api_new_branch)
 

      api_commit_flags_doc = load_doc(project.api_commit_flags)
 

      api_commit_add_flag_doc = load_doc(project.api_commit_add_flag)
 

+     api_update_project_watchers_doc = load_doc(
 

+         project.api_update_project_watchers)
 

  

      issues = []
 

      if pagure_config.get('ENABLE_TICKETS', True):
 
@@ -538,6 +540,7 @@ 
 

              api_new_branch_doc,
 

              api_commit_flags_doc,
 

              api_commit_add_flag_doc,
 

+             api_update_project_watchers_doc,
 

          ],
 

          issues=issues,
 

          requests=[

file modified
+134 
@@ -1580,3 +1580,137 @@ 
 

  

      jsonout = flask.jsonify(output)
 

      return jsonout
 

+ 

+ 

+ @API.route('/<repo>/watchers/update', methods=['POST'])
 

+ @API.route('/<namespace>/<repo>/watchers/update', methods=['POST'])
 

+ @API.route('/fork/<username>/<repo>/watchers/update', methods=['POST'])
 

+ @API.route(
 

+     '/fork/<username>/<namespace>/<repo>/watchers/update', methods=['POST'])
 

+ @api_login_required(acls=['update_watch_status'])
 

+ @api_method
 

+ def api_update_project_watchers(repo, username=None, namespace=None):
 

+     '''
 

+     Update project watchers
 

+     -----------------------
 

+     Allows anyone to update their own watch status on the project.
 

+ 

+     ::
 

+ 

+         POST /api/0/<repo>/watchers/update
 

+         POST /api/0/<namespace>/<repo>/watchers/update
 

+ 

+     ::
 

+ 

+         POST /api/0/fork/<username>/<repo>/watchers/update
 

+         POST /api/0/fork/<username>/<namespace>/<repo>/watchers/update
 

+ 

+     Input
 

+     ^^^^^
 

+ 

+     +------------------+---------+--------------+---------------------------+
 

+     | Key              | Type    | Optionality  | Description               |
 

+     +==================+=========+==============+===========================+
 

+     | ``repo``         | string  | Mandatory    | | The name of the project |
 

+     |                  |         |              |   to fork.                |
 

+     +------------------+---------+--------------+---------------------------+
 

+     | ``status``       | string  | Mandatory    | | The new watch status to |
 

+     |                  |         |              |   set on that project.    |
 

+     |                  |         |              |   (See options below)     |
 

+     +------------------+---------+--------------+---------------------------+
 

+     | ``watcher``      | string  | Mandatory    | | The name of the user    |
 

+     |                  |         |              |   changing their watch    |
 

+     |                  |         |              |   status.                 |
 

+     +------------------+---------+--------------+---------------------------+
 

+     | ``namespace``    | string  | Optional     | | The namespace of the    |
 

+     |                  |         |              |   project to fork.        |
 

+     +------------------+---------+--------------+---------------------------+
 

+     | ``username``     | string  | Optional     | | The username of the user|
 

+     |                  |         |              |   of the fork.            |
 

+     +------------------+---------+--------------+---------------------------+
 

+ 

+     Watch Status
 

+     ^^^^^^^^^^^^
 

+ 

+     +------------+----------------------------------------------+
 

+     | Key        | Description                                  |
 

+     +============+==============================================+
 

+     | -1         | Reset the watch status to default            |
 

+     +------------+----------------------------------------------+
 

+     | 0          | Unwatch, don't notify the user of anything   |
 

+     +------------+----------------------------------------------+
 

+     | 1          | Watch issues and pull-requests               |
 

+     +------------+----------------------------------------------+
 

+     | 2          | Watch commits                                |
 

+     +------------+----------------------------------------------+
 

+     | 3          | Watch commits, issues and pull-requests      |
 

+     +------------+----------------------------------------------+
 

+ 

+     Sample response
 

+     ^^^^^^^^^^^^^^^
 

+ 

+     ::
 

+ 

+         {
 

+             "message": "You are now watching issues and PRs on this project",
 

+             "status": "ok"
 

+         }
 

+     '''
 

+ 

+     project = get_authorized_api_project(
 

+         flask.g.session, repo, namespace=namespace)
 

+     if not project:
 

+         raise pagure.exceptions.APIError(
 

+             404, error_code=APIERROR.ENOPROJECT)
 

+ 

+     if flask.g.token.project and project != flask.g.token.project:
 

+         raise pagure.exceptions.APIError(
 

+             401, error_code=APIERROR.EINVALIDTOK)
 

+ 

+     # Get the input submitted
 

+     data = get_request_data()
 

+ 

+     watcher = data.get('watcher')
 

+ 

+     if not watcher:
 

+         _log.debug(
 

+             'api_update_project_watchers: Invalid watcher: %s',
 

+             watcher)
 

+         raise pagure.exceptions.APIError(
 

+             400, error_code=APIERROR.EINVALIDREQ)
 

+ 

+     is_site_admin = pagure.utils.is_admin()
 

+     # Only allow the main admin, and the user themselves to update their
 

+     # status
 

+     if not is_site_admin and flask.g.fas_user.username != watcher:
 

+         raise pagure.exceptions.APIError(
 

+             401, error_code=APIERROR.EMODIFYPROJECTNOTALLOWED)
 

+ 

+     try:
 

+         pagure.lib.get_user(flask.g.session, watcher)
 

+     except pagure.exceptions.PagureException as err:
 

+         _log.debug(
 

+             'api_update_project_watchers: Invalid user watching: %s',
 

+             watcher)
 

+         raise pagure.exceptions.APIError(
 

+             400, error_code=APIERROR.EINVALIDREQ)
 

+ 

+     watch_status = data.get('status')
 

+ 

+     try:
 

+         msg = pagure.lib.update_watch_status(
 

+             session=flask.g.session,
 

+             project=project,
 

+             user=watcher,
 

+             watch=watch_status)
 

+         flask.g.session.commit()
 

+     except pagure.exceptions.PagureException as err:
 

+         raise pagure.exceptions.APIError(
 

+             400, error_code=APIERROR.ENOCODE, error=str(err))
 

+     except SQLAlchemyError as err:  # pragma: no cover
 

+         flask.g.session.rollback()
 

+         _log.exception(err)
 

+         raise pagure.exceptions.APIError(
 

+             400, error_code=APIERROR.EDBERROR)
 

+ 

+     return flask.jsonify({'message': msg, 'status': 'ok'})

file modified
+3 -1 
@@ -289,6 +289,7 @@ 
 

      'pull_request_merge': 'Merge a pull-request',
 

      'pull_request_subscribe':
 

          'Subscribe the user with this token to a pull-request',
 

+     'update_watch_status': 'Update the watch status on a project',
 

  }
 

  

  # List of ACLs which a regular user is allowed to associate to an API token
 
@@ -300,7 +301,8 @@ 
 

  CROSS_PROJECT_ACLS = [
 

      'create_project',
 

      'fork_project',
 

-     'modify_project'
 

+     'modify_project',
 

+     'update_watch_status',
 

  ]
 

  

  # ACLs with which admins are allowed to create project-less API tokens

file added
+299 
@@ -0,0 +1,299 @@ 
 

+ # -*- coding: utf-8 -*-
 

+ 

+ """
 

+  (c) 2018 - Copyright Red Hat Inc
 

+ 

+  Authors:
 

+    Pierre-Yves Chibon <pingou@pingoured.fr>
 

+ 

+ """
 

+ 

+ from __future__ import unicode_literals
 

+ 

+ __requires__ = ['SQLAlchemy >= 0.8']
 

+ import pkg_resources
 

+ 

+ import copy
 

+ import datetime
 

+ import unittest
 

+ import shutil
 

+ import sys
 

+ import time
 

+ import os
 

+ 

+ import json
 

+ from mock import patch, MagicMock
 

+ 

+ sys.path.insert(0, os.path.join(os.path.dirname(
 

+     os.path.abspath(__file__)), '..'))
 

+ 

+ import pagure
 

+ import pagure.lib
 

+ import tests
 

+ 

+ 

+ class PagureFlaskApiProjectUpdateWatchTests(tests.Modeltests):
 

+     """ Tests for the flask API of pagure for changing the watch status on
 

+     a project via the API
 

+     """
 

+ 

+     @patch('pagure.lib.notify.send_email', MagicMock(return_value=True))
 

+     def setUp(self):
 

+         """ Set up the environnment, ran before every tests. """
 

+         super(PagureFlaskApiProjectUpdateWatchTests, self).setUp()
 

+ 

+         tests.create_projects(self.session)
 

+         tests.create_projects_git(os.path.join(self.path, 'tickets'))
 

+         tests.create_tokens(self.session)
 

+         tests.create_tokens_acl(self.session)
 

+ 

+         # Create normal issue
 

+         repo = pagure.lib.get_authorized_project(self.session, 'test')
 

+         msg = pagure.lib.new_issue(
 

+             session=self.session,
 

+             repo=repo,
 

+             title='Test issue #1',
 

+             content='We should work on this',
 

+             user='pingou',
 

+             ticketfolder=None,
 

+             private=False,
 

+         )
 

+         self.session.commit()
 

+         self.assertEqual(msg.title, 'Test issue #1')
 

+ 

+         # Create project-less token for user foo
 

+         item = pagure.lib.model.Token(
 

+             id='project-less-foo',
 

+             user_id=1,
 

+             project_id=None,
 

+             expiration=datetime.datetime.utcnow()
 

+             + datetime.timedelta(days=30)
 

+         )
 

+         self.session.add(item)
 

+         self.session.commit()
 

+         tests.create_tokens_acl(self.session, token_id='project-less-foo')
 

+ 

+     def test_api_update_project_watchers_invalid_project(self):
 

+         """ Test the api_update_project_watchers method of the flask api. """
 

+ 

+         headers = {'Authorization': 'token aaabbbcccddd'}
 

+ 

+         # Invalid project
 

+         output = self.app.post(
 

+             '/api/0/foobar/watchers/update', headers=headers)
 

+         self.assertEqual(output.status_code, 404)
 

+         data = json.loads(output.get_data(as_text=True))
 

+         self.assertDictEqual(
 

+             data,
 

+             {
 

+               "error": "Project not found",
 

+               "error_code": "ENOPROJECT",
 

+             }
 

+         )
 

+ 

+     def test_api_change_status_issue_token_not_for_project(self):
 

+         """ Test the api_update_project_watchers method of the flask api. """
 

+ 

+         headers = {'Authorization': 'token aaabbbcccddd'}
 

+ 

+         # Valid token, wrong project
 

+         output = self.app.post(
 

+             '/api/0/test2/watchers/update', headers=headers)
 

+         self.assertEqual(output.status_code, 401)
 

+         data = json.loads(output.get_data(as_text=True))
 

+         self.assertEqual(pagure.api.APIERROR.EINVALIDTOK.name,
 

+                          data['error_code'])
 

+         self.assertEqual(pagure.api.APIERROR.EINVALIDTOK.value, data['error'])
 

+ 

+     def test_api_update_project_watchers_no_user_watching(self):
 

+         """ Test the api_update_project_watchers method of the flask api. """
 

+ 

+         headers = {'Authorization': 'token aaabbbcccddd'}
 

+         data = {
 

+             'status': '42',
 

+         }
 

+ 

+         output = self.app.post(
 

+             '/api/0/test/watchers/update', headers=headers, data=data)
 

+         self.assertEqual(output.status_code, 400)
 

+         data = json.loads(output.get_data(as_text=True))
 

+         self.assertDictEqual(
 

+             data,
 

+             {
 

+                 u'error': u'Invalid or incomplete input submitted',
 

+                 u'error_code': u'EINVALIDREQ'
 

+             }
 

+         )
 

+ 

+     def test_api_update_project_watchers_no_watch_status(self):
 

+         """ Test the api_update_project_watchers method of the flask api. """
 

+ 

+         headers = {'Authorization': 'token aaabbbcccddd'}
 

+         data = {
 

+             'watcher': 'pingou',
 

+         }
 

+ 

+         output = self.app.post(
 

+             '/api/0/test/watchers/update', headers=headers, data=data)
 

+         self.assertEqual(output.status_code, 400)
 

+         data = json.loads(output.get_data(as_text=True))
 

+         self.assertDictEqual(
 

+             data,
 

+             {
 

+                 u'error': u'The watch value of "None" is invalid',
 

+                 u'error_code': u'ENOCODE'
 

+             }
 

+         )
 

+ 

+     def test_api_update_project_watchers_invalid_status(self):
 

+         """ Test the api_update_project_watchers method of the flask api. """
 

+ 

+         headers = {'Authorization': 'token aaabbbcccddd'}
 

+         data = {
 

+             'watcher': 'pingou',
 

+             'status': '42',
 

+         }
 

+ 

+         output = self.app.post(
 

+             '/api/0/test/watchers/update', headers=headers, data=data)
 

+         self.assertEqual(output.status_code, 400)
 

+         data = json.loads(output.get_data(as_text=True))
 

+         self.assertDictEqual(
 

+             data,
 

+             {
 

+                 u'error': u'The watch value of "42" is invalid',
 

+                 u'error_code': u'ENOCODE'
 

+             }
 

+         )
 

+ 

+     def test_api_update_project_watchers_invalid_user(self):
 

+         """ Test the api_update_project_watchers method of the flask api. """
 

+ 

+         headers = {'Authorization': 'token aaabbbcccddd'}
 

+         data = {
 

+             'watcher': 'example',
 

+             'status': '2',
 

+         }
 

+ 

+         output = self.app.post(
 

+             '/api/0/test/watchers/update', headers=headers, data=data)
 

+         self.assertEqual(output.status_code, 401)
 

+         data = json.loads(output.get_data(as_text=True))
 

+         self.assertDictEqual(
 

+             data,
 

+             {
 

+                 u'error': u'You are not allowed to modify this project',
 

+                 u'error_code': u'EMODIFYPROJECTNOTALLOWED'
 

+             }
 

+         )
 

+ 

+     def test_api_update_project_watchers_other_user(self):
 

+         """ Test the api_update_project_watchers method of the flask api. """
 

+ 

+         headers = {'Authorization': 'token aaabbbcccddd'}
 

+         data = {
 

+             'watcher': 'foo',
 

+             'status': '2',
 

+         }
 

+ 

+         output = self.app.post(
 

+             '/api/0/test/watchers/update', headers=headers, data=data)
 

+         self.assertEqual(output.status_code, 401)
 

+         data = json.loads(output.get_data(as_text=True))
 

+         self.assertDictEqual(
 

+             data,
 

+             {
 

+                 u'error': u'You are not allowed to modify this project',
 

+                 u'error_code': u'EMODIFYPROJECTNOTALLOWED'
 

+             }
 

+         )
 

+ 

+     def test_api_update_project_watchers_all_good(self):
 

+         """ Test the api_update_project_watchers method of the flask api. """
 

+ 

+         headers = {'Authorization': 'token aaabbbcccddd'}
 

+         data = {
 

+             'watcher': 'pingou',
 

+             'status': 1,
 

+         }
 

+ 

+         output = self.app.post(
 

+             '/api/0/test/watchers/update', headers=headers, data=data)
 

+         self.assertEqual(output.status_code, 200)
 

+         data = json.loads(output.get_data(as_text=True))
 

+         self.assertDictEqual(
 

+             data,
 

+             {
 

+                 u'message': u'You are now watching issues and PRs on this project',
 

+                 u'status': u'ok'
 

+             }
 

+         )
 

+ 

+     @patch('pagure.utils.is_admin', MagicMock(return_value=True))
 

+     def test_api_update_project_watchers_other_user_admin(self):
 

+         """ Test the api_update_project_watchers method of the flask api. """
 

+ 

+         headers = {'Authorization': 'token aaabbbcccddd'}
 

+         data = {
 

+             'watcher': 'foo',
 

+             'status': '2',
 

+         }
 

+ 

+         output = self.app.post(
 

+             '/api/0/test/watchers/update', headers=headers, data=data)
 

+         self.assertEqual(output.status_code, 200)
 

+         data = json.loads(output.get_data(as_text=True))
 

+         self.assertDictEqual(
 

+             data,
 

+             {
 

+                 u'message': u'You are now watching commits on this project',
 

+                 u'status': u'ok'
 

+             }
 

+         )
 

+ 

+     @patch('pagure.utils.is_admin', MagicMock(return_value=True))
 

+     def test_api_update_project_watchers_invalid_user_admin(self):
 

+         """ Test the api_update_project_watchers method of the flask api. """
 

+ 

+         headers = {'Authorization': 'token aaabbbcccddd'}
 

+         data = {
 

+             'watcher': 'example',
 

+             'status': '2',
 

+         }
 

+ 

+         output = self.app.post(
 

+             '/api/0/test/watchers/update', headers=headers, data=data)
 

+         self.assertEqual(output.status_code, 400)
 

+         data = json.loads(output.get_data(as_text=True))
 

+         self.assertDictEqual(
 

+             data,
 

+             {
 

+                 u'error': u'Invalid or incomplete input submitted',
 

+                 u'error_code': u'EINVALIDREQ'
 

+             }
 

+         )
 

+ 

+     @patch('pagure.utils.is_admin', MagicMock(return_value=True))
 

+     def test_api_update_project_watchers_missing_user_admin(self):
 

+         """ Test the api_update_project_watchers method of the flask api. """
 

+ 

+         headers = {'Authorization': 'token aaabbbcccddd'}
 

+         data = {
 

+             'status': '2',
 

+         }
 

+ 

+         output = self.app.post(
 

+             '/api/0/test/watchers/update', headers=headers, data=data)
 

+         self.assertEqual(output.status_code, 400)
 

+         data = json.loads(output.get_data(as_text=True))
 

+         self.assertDictEqual(
 

+             data,
 

+             {
 

+                 u'error': u'Invalid or incomplete input submitted',
 

+                 u'error_code': u'EINVALIDREQ'
 

+             }
 

+         )
 

+ 

+ 

+ if __name__ == '__main__':
 

+     unittest.main(verbosity=2)

file modified
+1 
@@ -5614,6 +5614,7 @@ 
 

                  'pull_request_flag',
 

                  'pull_request_merge',
 

                  'pull_request_subscribe',
 

+                 'update_watch_status',
 

              ]
 

          )

With this people with the appropriate API token will be able to adjust
their own watch status and instance-wide admins will be able to do it
for any user.

Fixes https://pagure.io/pagure/issue/3205
Fixes https://pagure.io/pagure/issue/3174

Signed-off-by: Pierre-Yves Chibon pingou@pingoured.fr

rebased onto f7333c9e3b9ff4b960654d293e5fc23c0a38cbd0
5 years ago

rebased onto 6e1b1fb1dcdebca33b63ffed3c0b3eb9a1583c23
5 years ago

Maybe we could use status and user here, I feel that watch_status and user_watching is a little bit redundant, since the endpoint name is giving the needed context.

should it be 3 here ? :)

should this be check just after line 94 ?

should we raise an invalid request exception here if watch_status is None ?

A few comments but looks good. Oh and some tests are failing :P
Edited 5 years ago by cverna

Agreed, but I wanted to distinguish the user whose watch status is being changed vs the username of the user who forked the project and I felt like having two fields: user and username may be too close to each other

Quite possibly:)

We could

pagure.lib.update_watch_status() takes care of that for us :)
(avoids duplicating code)

oh yes, that's a valid point, since the api endpoint is watchers/update maybe watcher would work ? but feel free to keep it as it is :)

+1

watcher and status it is :)

rebased onto acaf1443b65a26ede58f130c6988ad067dda9eba
5 years ago

LGTM :)

rebased onto 78dff61a320fb79b14d5e790c3815daabdb3470e
5 years ago

Jenkins caught an typo :)

rebased onto 12748fcd30ab84fbb9269cf8d781ad2928d4fc92
5 years ago

rebased onto f778bdc
5 years ago

Pretty please pagure-ci rebuild

+1

Thanks for the review :)

Pull-Request has been merged by pingou
5 years ago
Metadata
None
No Tags
Flags
jenkins #440
success (100%)
Build successful (commit: f778bdca)
5 years ago
jenkins #440
success (100%)
Build successful (commit: f778bdca)
5 years ago
jenkins #437
failure
Build failed (commit: f778bdca)
5 years ago
jenkins #437
failure
Build failed (commit: f778bdca)
5 years ago
jenkins #431
failure
Build failed (commit: f778bdca)
5 years ago
jenkins #431
failure
Build failed (commit: f778bdca)
5 years ago
jenkins #423
failure
Build failed (commit: f778bdca)
5 years ago
jenkins #423
failure
Build failed (commit: f778bdca)
5 years ago
jenkins #420
failure
Build failed (commit: f778bdca)
5 years ago
jenkins #420
failure
Build failed (commit: f778bdca)
5 years ago
jenkins #446
success (100%)
Build successful
5 years ago
jenkins #445
failure
Build failed
5 years ago
jenkins #444
failure
Build failed
5 years ago
jenkins #437
failure
Build failed
5 years ago
jenkins #431
failure
Build failed
5 years ago
jenkins #423
failure
Build failed
5 years ago
jenkins #420
failure
Build failed
5 years ago
Changes Summary 5
+3 -0
file changed
pagure/api/__init__.py
+134 -0
file changed
pagure/api/project.py
+3 -1
file changed
pagure/default_config.py
+299
file added
tests/test_pagure_flask_api_project_update_watch.py
+1 -0
file changed
tests/test_pagure_lib.py
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.