diff mbox series

Fix issue with delegation of patch via REST API

Message ID 20190921183046.27279-1-stephen@that.guru
State Accepted
Headers show
Series Fix issue with delegation of patch via REST API | expand

Commit Message

Stephen Finucane Sept. 21, 2019, 6:30 p.m. UTC
There have been reports of people being unable to delegate patches to
themselves, despite being a maintainer or the project to which the patch
is associated.

The issue is a result of how we do a check for whether the user is a
maintainer of the patch's project [1]. This check is checking if a given
'User.id' is in the list of items referenced by
'Project.maintainer_project'. However, 'Project.maintainer_project' is a
backref to 'UserProfile.maintainer_projects'. This means we're comparing
'User.id' and 'UserProfile.id'. Boo.

This wasn't seen in testing since we've had a post-save callback [2] for some
time that ensures we always create a 'UserProfile' object whenever we create a
'User' object. This also means we won't have an issue on deployments initially
deployed after that post-save callback was added, a 'User' with id=N will
always have a corresponding 'UserProfile' with id=N. However, that's not true
for older deployments such as the ozlabs.org one.

[1] https://github.com/getpatchwork/patchwork/blob/89c924f9bc/patchwork/api/patch.py#L108-L111
[2] https://github.com/getpatchwork/patchwork/blob/89c924f9bc/patchwork/models.py#L204-L210

Signed-off-by: Stephen Finucane <stephen@that.guru>
Closes: #313
Reported-by: Bjorn Helgaas <helgaas@kernel.org>
---
 patchwork/api/patch.py            |  4 ++--
 patchwork/tests/api/test_patch.py | 24 ++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)

Comments

Bjorn Helgaas Sept. 23, 2019, 8:23 p.m. UTC | #1
[+cc Jeremy, Michael]

On Sat, Sep 21, 2019 at 07:30:46PM +0100, Stephen Finucane wrote:
> There have been reports of people being unable to delegate patches to
> themselves, despite being a maintainer or the project to which the patch
> is associated.
> 
> The issue is a result of how we do a check for whether the user is a
> maintainer of the patch's project [1]. This check is checking if a given
> 'User.id' is in the list of items referenced by
> 'Project.maintainer_project'. However, 'Project.maintainer_project' is a
> backref to 'UserProfile.maintainer_projects'. This means we're comparing
> 'User.id' and 'UserProfile.id'. Boo.
> 
> This wasn't seen in testing since we've had a post-save callback [2] for some
> time that ensures we always create a 'UserProfile' object whenever we create a
> 'User' object. This also means we won't have an issue on deployments initially
> deployed after that post-save callback was added, a 'User' with id=N will
> always have a corresponding 'UserProfile' with id=N. However, that's not true
> for older deployments such as the ozlabs.org one.

Thanks a lot for fixing this so fast.

I don't know who runs patchwork.ozlabs.org, maybe Jeremy or Michael
has a pointer?  And I don't know if/when your fix might show up in
that instance.  It sounds like maybe there's a possibility of a manual
server-side workaround in the meantime, i.e., creating a UserProfile
matching my User?

Or maybe a more involved client-side workaround that uses whatever
path http://patchwork.ozlabs.org/ uses for delegation?
Bjorn Helgaas Sept. 23, 2019, 8:39 p.m. UTC | #2
On Sat, Sep 21, 2019 at 07:30:46PM +0100, Stephen Finucane wrote:
> There have been reports of people being unable to delegate patches to
> themselves, despite being a maintainer or the project to which the patch
> is associated.
> 
> The issue is a result of how we do a check for whether the user is a
> maintainer of the patch's project [1]. This check is checking if a given
> 'User.id' is in the list of items referenced by
> 'Project.maintainer_project'. However, 'Project.maintainer_project' is a
> backref to 'UserProfile.maintainer_projects'. This means we're comparing
> 'User.id' and 'UserProfile.id'. Boo.
> 
> This wasn't seen in testing since we've had a post-save callback [2] for some
> time that ensures we always create a 'UserProfile' object whenever we create a
> 'User' object. This also means we won't have an issue on deployments initially
> deployed after that post-save callback was added, a 'User' with id=N will
> always have a corresponding 'UserProfile' with id=N. However, that's not true
> for older deployments such as the ozlabs.org one.

I tried the instance at https://patchwork.kernel.org/project/linux-pci/list/
to see if it was new enough to work without this fix.  But it also
fails, slightly differently:

  $ git config -l | grep "^pw"
  pw.server=https://patchwork.kernel.org/api/1.1
  pw.project=linux-pci
  pw.token=...

  $ git-pw patch update --delegate helgaas 11151519
  More than one delegate found: helgaas

Is this another manifestation of the same bug or something else?

> [1] https://github.com/getpatchwork/patchwork/blob/89c924f9bc/patchwork/api/patch.py#L108-L111
> [2] https://github.com/getpatchwork/patchwork/blob/89c924f9bc/patchwork/models.py#L204-L210
> 
> Signed-off-by: Stephen Finucane <stephen@that.guru>
> Closes: #313
> Reported-by: Bjorn Helgaas <helgaas@kernel.org>
> ---
>  patchwork/api/patch.py            |  4 ++--
>  patchwork/tests/api/test_patch.py | 24 ++++++++++++++++++++++++
>  2 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py
> index c9360308..d1c9904d 100644
> --- a/patchwork/api/patch.py
> +++ b/patchwork/api/patch.py
> @@ -105,8 +105,8 @@ class PatchListSerializer(BaseHyperlinkedModelSerializer):
>          if not value:
>              return value
>  
> -        if not self.instance.project.maintainer_project.filter(
> -                id=value.id).exists():
> +        if not value.profile.maintainer_projects.only('id').filter(
> +                id=self.instance.project.id).exists():
>              raise ValidationError("User '%s' is not a maintainer for project "
>                                    "'%s'" % (value, self.instance.project))
>          return value
> diff --git a/patchwork/tests/api/test_patch.py b/patchwork/tests/api/test_patch.py
> index 82ae0184..edae9851 100644
> --- a/patchwork/tests/api/test_patch.py
> +++ b/patchwork/tests/api/test_patch.py
> @@ -284,6 +284,30 @@ class TestPatchAPI(utils.APITestCase):
>          self.assertContains(resp, 'Expected one of: %s.' % state.name,
>                              status_code=status.HTTP_400_BAD_REQUEST)
>  
> +    def test_update_legacy_delegate(self):
> +        """Regression test for bug #313."""
> +        project = create_project()
> +        state = create_state()
> +        patch = create_patch(project=project, state=state)
> +        user_a = create_maintainer(project)
> +
> +        # create a user (User), then delete the associated UserProfile and save
> +        # the user to ensure a new profile is generated
> +        user_b = create_user()
> +        self.assertEqual(user_b.id, user_b.profile.id)
> +        user_b.profile.delete()
> +        user_b.save()
> +        user_b.profile.maintainer_projects.add(project)
> +        user_b.profile.save()
> +        self.assertNotEqual(user_b.id, user_b.profile.id)
> +
> +        self.client.force_authenticate(user=user_a)
> +        resp = self.client.patch(self.api_url(patch.id),
> +                                 {'delegate': user_b.id})
> +        self.assertEqual(status.HTTP_200_OK, resp.status_code, resp)
> +        self.assertEqual(Patch.objects.get(id=patch.id).state, state)
> +        self.assertEqual(Patch.objects.get(id=patch.id).delegate, user_b)
> +
>      def test_update_invalid_delegate(self):
>          """Update patch with invalid fields.
>  
> -- 
> 2.21.0
>
Stephen Finucane Sept. 24, 2019, 8:45 a.m. UTC | #3
On Mon, 2019-09-23 at 15:39 -0500, Bjorn Helgaas wrote:
> On Sat, Sep 21, 2019 at 07:30:46PM +0100, Stephen Finucane wrote:
> > There have been reports of people being unable to delegate patches to
> > themselves, despite being a maintainer or the project to which the patch
> > is associated.
> > 
> > The issue is a result of how we do a check for whether the user is a
> > maintainer of the patch's project [1]. This check is checking if a given
> > 'User.id' is in the list of items referenced by
> > 'Project.maintainer_project'. However, 'Project.maintainer_project' is a
> > backref to 'UserProfile.maintainer_projects'. This means we're comparing
> > 'User.id' and 'UserProfile.id'. Boo.
> > 
> > This wasn't seen in testing since we've had a post-save callback [2] for some
> > time that ensures we always create a 'UserProfile' object whenever we create a
> > 'User' object. This also means we won't have an issue on deployments initially
> > deployed after that post-save callback was added, a 'User' with id=N will
> > always have a corresponding 'UserProfile' with id=N. However, that's not true
> > for older deployments such as the ozlabs.org one.
> 
> I tried the instance at https://patchwork.kernel.org/project/linux-pci/list/
> to see if it was new enough to work without this fix.  But it also
> fails, slightly differently:
> 
>   $ git config -l | grep "^pw"
>   pw.server=https://patchwork.kernel.org/api/1.1
>   pw.project=linux-pci
>   pw.token=...
> 
>   $ git-pw patch update --delegate helgaas 11151519
>   More than one delegate found: helgaas
> 
> Is this another manifestation of the same bug or something else?

This is a different issue and, unlike the other one, is more feature
than bug. This is happening because the search for a particular user is
returning multiple matches. We match on username, first name, last name
and email, so I imagine you have multiple user accounts on the instance
and there might be a conflict between an email address of one account
and a username of another? (Let me know if this isn't the case). The
easy solution is to use a more specific match. I'd suggest just using
the email address associated with your user account ([1] suggests this
is 'bhelgaas@google.com'). We could also support lookup by user ID
(which would guarantee a single match) but I haven't added that to git-
pw yet since it didn't seem that usable.

Stephen

[1] https://patchwork.kernel.org/project/linux-pci/

> > [1] https://github.com/getpatchwork/patchwork/blob/89c924f9bc/patchwork/api/patch.py#L108-L111
> > [2] https://github.com/getpatchwork/patchwork/blob/89c924f9bc/patchwork/models.py#L204-L210
> > 
> > Signed-off-by: Stephen Finucane <stephen@that.guru>
> > Closes: #313
> > Reported-by: Bjorn Helgaas <helgaas@kernel.org>
> > ---
> >  patchwork/api/patch.py            |  4 ++--
> >  patchwork/tests/api/test_patch.py | 24 ++++++++++++++++++++++++
> >  2 files changed, 26 insertions(+), 2 deletions(-)
> > 
> > diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py
> > index c9360308..d1c9904d 100644
> > --- a/patchwork/api/patch.py
> > +++ b/patchwork/api/patch.py
> > @@ -105,8 +105,8 @@ class PatchListSerializer(BaseHyperlinkedModelSerializer):
> >          if not value:
> >              return value
> >  
> > -        if not self.instance.project.maintainer_project.filter(
> > -                id=value.id).exists():
> > +        if not value.profile.maintainer_projects.only('id').filter(
> > +                id=self.instance.project.id).exists():
> >              raise ValidationError("User '%s' is not a maintainer for project "
> >                                    "'%s'" % (value, self.instance.project))
> >          return value
> > diff --git a/patchwork/tests/api/test_patch.py b/patchwork/tests/api/test_patch.py
> > index 82ae0184..edae9851 100644
> > --- a/patchwork/tests/api/test_patch.py
> > +++ b/patchwork/tests/api/test_patch.py
> > @@ -284,6 +284,30 @@ class TestPatchAPI(utils.APITestCase):
> >          self.assertContains(resp, 'Expected one of: %s.' % state.name,
> >                              status_code=status.HTTP_400_BAD_REQUEST)
> >  
> > +    def test_update_legacy_delegate(self):
> > +        """Regression test for bug #313."""
> > +        project = create_project()
> > +        state = create_state()
> > +        patch = create_patch(project=project, state=state)
> > +        user_a = create_maintainer(project)
> > +
> > +        # create a user (User), then delete the associated UserProfile and save
> > +        # the user to ensure a new profile is generated
> > +        user_b = create_user()
> > +        self.assertEqual(user_b.id, user_b.profile.id)
> > +        user_b.profile.delete()
> > +        user_b.save()
> > +        user_b.profile.maintainer_projects.add(project)
> > +        user_b.profile.save()
> > +        self.assertNotEqual(user_b.id, user_b.profile.id)
> > +
> > +        self.client.force_authenticate(user=user_a)
> > +        resp = self.client.patch(self.api_url(patch.id),
> > +                                 {'delegate': user_b.id})
> > +        self.assertEqual(status.HTTP_200_OK, resp.status_code, resp)
> > +        self.assertEqual(Patch.objects.get(id=patch.id).state, state)
> > +        self.assertEqual(Patch.objects.get(id=patch.id).delegate, user_b)
> > +
> >      def test_update_invalid_delegate(self):
> >          """Update patch with invalid fields.
> >  
> > -- 
> > 2.21.0
> >
Stephen Finucane Sept. 24, 2019, 8:51 a.m. UTC | #4
On Mon, 2019-09-23 at 15:23 -0500, Bjorn Helgaas wrote:
> [+cc Jeremy, Michael]
> 
> On Sat, Sep 21, 2019 at 07:30:46PM +0100, Stephen Finucane wrote:
> > There have been reports of people being unable to delegate patches to
> > themselves, despite being a maintainer or the project to which the patch
> > is associated.
> > 
> > The issue is a result of how we do a check for whether the user is a
> > maintainer of the patch's project [1]. This check is checking if a given
> > 'User.id' is in the list of items referenced by
> > 'Project.maintainer_project'. However, 'Project.maintainer_project' is a
> > backref to 'UserProfile.maintainer_projects'. This means we're comparing
> > 'User.id' and 'UserProfile.id'. Boo.
> > 
> > This wasn't seen in testing since we've had a post-save callback [2] for some
> > time that ensures we always create a 'UserProfile' object whenever we create a
> > 'User' object. This also means we won't have an issue on deployments initially
> > deployed after that post-save callback was added, a 'User' with id=N will
> > always have a corresponding 'UserProfile' with id=N. However, that's not true
> > for older deployments such as the ozlabs.org one.
> 
> Thanks a lot for fixing this so fast.
> 
> I don't know who runs patchwork.ozlabs.org, maybe Jeremy or Michael
> has a pointer?  And I don't know if/when your fix might show up in
> that instance.  It sounds like maybe there's a possibility of a manual
> server-side workaround in the meantime, i.e., creating a UserProfile
> matching my User?

That won't work because you'd have to modify UserProfile.id to match
the corresponding User.id for every entry. Definitely not worth it.

> Or maybe a more involved client-side workaround that uses whatever
> path http://patchwork.ozlabs.org/ uses for delegation?

I've been considering that but the problem is we don't expose user
profile information via the API. This makes sense because the fields on
the user profile are not something you'd need/want to be globally
visable (mostly personal preferences for the web interface) but it
means we've no way to discover the userprofile. We're stuck waiting for
this fix, I'm afraid.

Stephen
Stephen Finucane Sept. 24, 2019, 9:10 a.m. UTC | #5
On Sat, 2019-09-21 at 19:30 +0100, Stephen Finucane wrote:
> There have been reports of people being unable to delegate patches to
> themselves, despite being a maintainer or the project to which the patch
> is associated.
> 
> The issue is a result of how we do a check for whether the user is a
> maintainer of the patch's project [1]. This check is checking if a given
> 'User.id' is in the list of items referenced by
> 'Project.maintainer_project'. However, 'Project.maintainer_project' is a
> backref to 'UserProfile.maintainer_projects'. This means we're comparing
> 'User.id' and 'UserProfile.id'. Boo.
> 
> This wasn't seen in testing since we've had a post-save callback [2] for some
> time that ensures we always create a 'UserProfile' object whenever we create a
> 'User' object. This also means we won't have an issue on deployments initially
> deployed after that post-save callback was added, a 'User' with id=N will
> always have a corresponding 'UserProfile' with id=N. However, that's not true
> for older deployments such as the ozlabs.org one.
> 
> [1] https://github.com/getpatchwork/patchwork/blob/89c924f9bc/patchwork/api/patch.py#L108-L111
> [2] https://github.com/getpatchwork/patchwork/blob/89c924f9bc/patchwork/models.py#L204-L210
> 
> Signed-off-by: Stephen Finucane <stephen@that.guru>
> Closes: #313
> Reported-by: Bjorn Helgaas <helgaas@kernel.org>

The test added here failed when the fix is reverted so I'm pretty
confident this is correct. As such, I've gone ahead and applied it and
will cut a v2.1.4 release later today.

Stephen
Stephen Finucane Sept. 24, 2019, 5:11 p.m. UTC | #6
On Tue, 2019-09-24 at 10:10 +0100, Stephen Finucane wrote:
> On Sat, 2019-09-21 at 19:30 +0100, Stephen Finucane wrote:
> > There have been reports of people being unable to delegate patches to
> > themselves, despite being a maintainer or the project to which the patch
> > is associated.
> > 
> > The issue is a result of how we do a check for whether the user is a
> > maintainer of the patch's project [1]. This check is checking if a given
> > 'User.id' is in the list of items referenced by
> > 'Project.maintainer_project'. However, 'Project.maintainer_project' is a
> > backref to 'UserProfile.maintainer_projects'. This means we're comparing
> > 'User.id' and 'UserProfile.id'. Boo.
> > 
> > This wasn't seen in testing since we've had a post-save callback [2] for some
> > time that ensures we always create a 'UserProfile' object whenever we create a
> > 'User' object. This also means we won't have an issue on deployments initially
> > deployed after that post-save callback was added, a 'User' with id=N will
> > always have a corresponding 'UserProfile' with id=N. However, that's not true
> > for older deployments such as the ozlabs.org one.
> > 
> > [1] https://github.com/getpatchwork/patchwork/blob/89c924f9bc/patchwork/api/patch.py#L108-L111
> > [2] https://github.com/getpatchwork/patchwork/blob/89c924f9bc/patchwork/models.py#L204-L210
> > 
> > Signed-off-by: Stephen Finucane <stephen@that.guru>
> > Closes: #313
> > Reported-by: Bjorn Helgaas <helgaas@kernel.org>
> 
> The test added here failed when the fix is reverted so I'm pretty
> confident this is correct. As such, I've gone ahead and applied it and
> will cut a v2.1.4 release later today.

v2.1.5 (!) has been released. Just got to get ozlabs up-to-speed now.

Stephen
Bjorn Helgaas Sept. 24, 2019, 7:12 p.m. UTC | #7
On Tue, Sep 24, 2019 at 09:45:16AM +0100, Stephen Finucane wrote:
> On Mon, 2019-09-23 at 15:39 -0500, Bjorn Helgaas wrote:
> > On Sat, Sep 21, 2019 at 07:30:46PM +0100, Stephen Finucane wrote:
> > > There have been reports of people being unable to delegate patches to
> > > themselves, despite being a maintainer or the project to which the patch
> > > is associated.
> > > ...

> > I tried the instance at https://patchwork.kernel.org/project/linux-pci/list/
> > to see if it was new enough to work without this fix.  But it also
> > fails, slightly differently:
> > 
> >   $ git config -l | grep "^pw"
> >   pw.server=https://patchwork.kernel.org/api/1.1
> >   pw.project=linux-pci
> >   pw.token=...
> > 
> >   $ git-pw patch update --delegate helgaas 11151519
> >   More than one delegate found: helgaas
> > 
> > Is this another manifestation of the same bug or something else?
> 
> This is a different issue and, unlike the other one, is more feature
> than bug. This is happening because the search for a particular user is
> returning multiple matches. We match on username, first name, last name
> and email, so I imagine you have multiple user accounts on the instance
> and there might be a conflict between an email address of one account
> and a username of another? (Let me know if this isn't the case). The
> easy solution is to use a more specific match. I'd suggest just using
> the email address associated with your user account ([1] suggests this
> is 'bhelgaas@google.com'). We could also support lookup by user ID
> (which would guarantee a single match) but I haven't added that to git-
> pw yet since it didn't seem that usable.

Still no workey.  I'm cursed.  The web "Delegate to" dropdown menu
includes "bhelgaas" and my profile seems to be associated with
"bhelgaas" (at least, that's the name in the upper right of the web
page when I'm logged in).

  $ git-pw patch update --delegate bhelgaas 11151519
  {u'delegate': [u"User 'bhelgaas' is not a maintainer for project 'Linux PCI development list'"]}
  $ git-pw patch update --delegate bhelgaas@google.com 11151519
  {u'delegate': [u"User 'bhelgaas' is not a maintainer for project 'Linux PCI development list'"]}
  $ git-pw patch update --delegate bjorn@helgaas.com 11151519
  No matching delegates found: bjorn@helgaas.com

https://patchwork.kernel.org/user/ also claims I'm a maintainer for
https://patchwork.kernel.org/project/linux-pci/list/ and shows both
bhelgaas@google.com and bjorn@helgaas.com as email addresses.

> [1] https://patchwork.kernel.org/project/linux-pci/
Michael Ellerman Sept. 25, 2019, 10:38 a.m. UTC | #8
On 24 September 2019 6:51:53 pm AEST, Stephen Finucane <stephen@that.guru> wrote:
>On Mon, 2019-09-23 at 15:23 -0500, Bjorn Helgaas wrote:
>> [+cc Jeremy, Michael]
>> 
>> On Sat, Sep 21, 2019 at 07:30:46PM +0100, Stephen Finucane wrote:
>> > There have been reports of people being unable to delegate patches
>to
>> > themselves, despite being a maintainer or the project to which the
>patch
>> > is associated.
>> > 
>> > The issue is a result of how we do a check for whether the user is
>a
>> > maintainer of the patch's project [1]. This check is checking if a
>given
>> > 'User.id' is in the list of items referenced by
>> > 'Project.maintainer_project'. However, 'Project.maintainer_project'
>is a
>> > backref to 'UserProfile.maintainer_projects'. This means we're
>comparing
>> > 'User.id' and 'UserProfile.id'. Boo.
>> > 
>> > This wasn't seen in testing since we've had a post-save callback
>[2] for some
>> > time that ensures we always create a 'UserProfile' object whenever
>we create a
>> > 'User' object. This also means we won't have an issue on
>deployments initially
>> > deployed after that post-save callback was added, a 'User' with
>id=N will
>> > always have a corresponding 'UserProfile' with id=N. However,
>that's not true
>> > for older deployments such as the ozlabs.org one.
>> 
>> Thanks a lot for fixing this so fast.
>> 
>> I don't know who runs patchwork.ozlabs.org, maybe Jeremy or Michael
>> has a pointer?  And I don't know if/when your fix might show up in
>> that instance.  It sounds like maybe there's a possibility of a
>manual
>> server-side workaround in the meantime, i.e., creating a UserProfile
>> matching my User?
>
>That won't work because you'd have to modify UserProfile.id to match
>the corresponding User.id for every entry. 

I don't follow that.

Bjorn's User id is 6763, if we create a profile with that id (it's free on ozlabs.org, I checked) wouldn't that allow him to delegate to himself?

cheers
Stephen Finucane Sept. 25, 2019, 3:33 p.m. UTC | #9
On Tue, 2019-09-24 at 14:12 -0500, Bjorn Helgaas wrote:
> On Tue, Sep 24, 2019 at 09:45:16AM +0100, Stephen Finucane wrote:
> > On Mon, 2019-09-23 at 15:39 -0500, Bjorn Helgaas wrote:
> > > On Sat, Sep 21, 2019 at 07:30:46PM +0100, Stephen Finucane wrote:
> > > > There have been reports of people being unable to delegate patches to
> > > > themselves, despite being a maintainer or the project to which the patch
> > > > is associated.
> > > > ...
> > > I tried the instance at https://patchwork.kernel.org/project/linux-pci/list/
> > > to see if it was new enough to work without this fix.  But it also
> > > fails, slightly differently:
> > > 
> > >   $ git config -l | grep "^pw"
> > >   pw.server=https://patchwork.kernel.org/api/1.1
> > >   pw.project=linux-pci
> > >   pw.token=...
> > > 
> > >   $ git-pw patch update --delegate helgaas 11151519
> > >   More than one delegate found: helgaas
> > > 
> > > Is this another manifestation of the same bug or something else?
> > 
> > This is a different issue and, unlike the other one, is more feature
> > than bug. This is happening because the search for a particular user is
> > returning multiple matches. We match on username, first name, last name
> > and email, so I imagine you have multiple user accounts on the instance
> > and there might be a conflict between an email address of one account
> > and a username of another? (Let me know if this isn't the case). The
> > easy solution is to use a more specific match. I'd suggest just using
> > the email address associated with your user account ([1] suggests this
> > is 'bhelgaas@google.com'). We could also support lookup by user ID
> > (which would guarantee a single match) but I haven't added that to git-
> > pw yet since it didn't seem that usable.
> 
> Still no workey.  I'm cursed.  The web "Delegate to" dropdown menu
> includes "bhelgaas" and my profile seems to be associated with
> "bhelgaas" (at least, that's the name in the upper right of the web
> page when I'm logged in).
> 
>   $ git-pw patch update --delegate bhelgaas 11151519
>   {u'delegate': [u"User 'bhelgaas' is not a maintainer for project 'Linux PCI development list'"]}
>   $ git-pw patch update --delegate bhelgaas@google.com 11151519
>   {u'delegate': [u"User 'bhelgaas' is not a maintainer for project 'Linux PCI development list'"]}
>   $ git-pw patch update --delegate bjorn@helgaas.com 11151519
>   No matching delegates found: bjorn@helgaas.com
> 
> https://patchwork.kernel.org/user/ also claims I'm a maintainer for
> https://patchwork.kernel.org/project/linux-pci/list/ and shows both
> bhelgaas@google.com and bjorn@helgaas.com as email addresses.

I'll have a look into this but I've no ideas off the top of my head. If
you're comfortable with Python, could you add a couple of print
statements to log what we're requesting from the API and what we're
getting back and share them here? If not, I'll try look into this next
week.

Stephen

> > [1] https://patchwork.kernel.org/project/linux-pci/
Bjorn Helgaas Sept. 25, 2019, 6:51 p.m. UTC | #10
On Wed, Sep 25, 2019 at 04:33:35PM +0100, Stephen Finucane wrote:
> On Tue, 2019-09-24 at 14:12 -0500, Bjorn Helgaas wrote:
> > On Tue, Sep 24, 2019 at 09:45:16AM +0100, Stephen Finucane wrote:
> > > On Mon, 2019-09-23 at 15:39 -0500, Bjorn Helgaas wrote:
> > > > On Sat, Sep 21, 2019 at 07:30:46PM +0100, Stephen Finucane wrote:
> > > > > There have been reports of people being unable to delegate patches to
> > > > > themselves, despite being a maintainer or the project to which the patch
> > > > > is associated.
> > > > > ...
> > > > I tried the instance at https://patchwork.kernel.org/project/linux-pci/list/
> > > > to see if it was new enough to work without this fix.  But it also
> > > > fails, slightly differently:
> > > > 
> > > >   $ git config -l | grep "^pw"
> > > >   pw.server=https://patchwork.kernel.org/api/1.1
> > > >   pw.project=linux-pci
> > > >   pw.token=...
> > > > 
> > > >   $ git-pw patch update --delegate helgaas 11151519
> > > >   More than one delegate found: helgaas
> > > > 
> > > > Is this another manifestation of the same bug or something else?
> > > 
> > > This is a different issue and, unlike the other one, is more feature
> > > than bug. This is happening because the search for a particular user is
> > > returning multiple matches. We match on username, first name, last name
> > > and email, so I imagine you have multiple user accounts on the instance
> > > and there might be a conflict between an email address of one account
> > > and a username of another? (Let me know if this isn't the case). The
> > > easy solution is to use a more specific match. I'd suggest just using
> > > the email address associated with your user account ([1] suggests this
> > > is 'bhelgaas@google.com'). We could also support lookup by user ID
> > > (which would guarantee a single match) but I haven't added that to git-
> > > pw yet since it didn't seem that usable.
> > 
> > Still no workey.  I'm cursed.  The web "Delegate to" dropdown menu
> > includes "bhelgaas" and my profile seems to be associated with
> > "bhelgaas" (at least, that's the name in the upper right of the web
> > page when I'm logged in).
> > 
> >   $ git-pw patch update --delegate bhelgaas 11151519
> >   {u'delegate': [u"User 'bhelgaas' is not a maintainer for project 'Linux PCI development list'"]}
> >   $ git-pw patch update --delegate bhelgaas@google.com 11151519
> >   {u'delegate': [u"User 'bhelgaas' is not a maintainer for project 'Linux PCI development list'"]}
> >   $ git-pw patch update --delegate bjorn@helgaas.com 11151519
> >   No matching delegates found: bjorn@helgaas.com
> > 
> > https://patchwork.kernel.org/user/ also claims I'm a maintainer for
> > https://patchwork.kernel.org/project/linux-pci/list/ and shows both
> > bhelgaas@google.com and bjorn@helgaas.com as email addresses.
> 
> I'll have a look into this but I've no ideas off the top of my head. If
> you're comfortable with Python, could you add a couple of print
> statements to log what we're requesting from the API and what we're
> getting back and share them here? If not, I'll try look into this next
> week.

I am able to change the *state*, e.g.,

  $ git-pw patch update --state accepted 11151519
  $ git-pw patch update --state new 11151519

seem to work fine.

Not sure if this has enough information to be useful to you, but this
is the output from:

  git-pw --debug patch update --delegate bhelgaas@google.com 11151519

2019-09-25 13:43:04,838 - git_pw.patch - DEBUG - Updating patch: id=11151519, commit_ref=None, state=None, archived=None
2019-09-25 13:43:04,843 - git_pw.config - DEBUG - Retrieved 'server' setting from git-config
2019-09-25 13:43:04,843 - git_pw.config - DEBUG - Retrieved 'server' setting from cache
2019-09-25 13:43:04,847 - git_pw.config - DEBUG - Retrieved 'project' setting from git-config
2019-09-25 13:43:04,847 - git_pw.config - DEBUG - Retrieved 'project' setting from cache
2019-09-25 13:43:04,847 - git_pw.config - DEBUG - Retrieved 'project' setting from cache
2019-09-25 13:43:04,847 - git_pw.config - DEBUG - Retrieved 'project' setting from cache
2019-09-25 13:43:04,847 - git_pw.api - DEBUG - GET https://patchwork.kernel.org/api/1.1/users/
2019-09-25 13:43:04,851 - git_pw.config - DEBUG - Retrieved 'token' setting from git-config
2019-09-25 13:43:04,852 - git_pw.config - DEBUG - Retrieved 'token' setting from cache
2019-09-25 13:43:04,853 - urllib3.connectionpool - DEBUG - Starting new HTTPS connection (1): patchwork.kernel.org:443
2019-09-25 13:43:05,414 - urllib3.connectionpool - DEBUG - https://patchwork.kernel.org:443 "GET /api/1.1/users/?q=bhelgaas%40google.com&project=linux-pci HTTP/1.1" 200 167
2019-09-25 13:43:05,415 - git_pw.api - DEBUG - Got response
2019-09-25 13:43:05,416 - git_pw.config - DEBUG - Retrieved 'server' setting from cache
2019-09-25 13:43:05,416 - git_pw.config - DEBUG - Retrieved 'server' setting from cache
2019-09-25 13:43:05,416 - git_pw.api - DEBUG - PATCH https://patchwork.kernel.org/api/1.1/patches/11151519/, data=[('delegate', 13257)]
2019-09-25 13:43:05,416 - git_pw.config - DEBUG - Retrieved 'token' setting from cache
2019-09-25 13:43:05,416 - git_pw.config - DEBUG - Retrieved 'token' setting from cache
2019-09-25 13:43:05,417 - urllib3.connectionpool - DEBUG - Starting new HTTPS connection (1): patchwork.kernel.org:443
2019-09-25 13:43:05,926 - urllib3.connectionpool - DEBUG - https://patchwork.kernel.org:443 "PATCH /api/1.1/patches/11151519/ HTTP/1.1" 400 93
2019-09-25 13:43:05,927 - git_pw.api - ERROR - JSON response
2019-09-25 13:43:05,928 - git_pw.api - ERROR - {u'delegate': [u"User 'bhelgaas' is not a maintainer for project 'Linux PCI development list'"]}
2019-09-25 13:43:05,928 - git_pw.config - DEBUG - Retrieved 'debug' setting from cache
Traceback (most recent call last):
  File "/home/bhelgaas/bin/git-pw", line 10, in <module>
    sys.exit(cli())
  File "/usr/lib/python2.7/dist-packages/click/core.py", line 764, in __call__
    return self.main(*args, **kwargs)
  File "/usr/lib/python2.7/dist-packages/click/core.py", line 717, in main
    rv = self.invoke(ctx)
  File "/usr/lib/python2.7/dist-packages/click/core.py", line 1137, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/usr/lib/python2.7/dist-packages/click/core.py", line 1137, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/usr/lib/python2.7/dist-packages/click/core.py", line 956, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/lib/python2.7/dist-packages/click/core.py", line 555, in invoke
    return callback(*args, **kwargs)
  File "/home/bhelgaas/src/git-pw/git_pw/patch.py", line 180, in update_cmd
    patch = api.update('patches', patch_id, data)
  File "/home/bhelgaas/src/git-pw/git_pw/api.py", line 277, in update
    return patch(url, data).json()
  File "/home/bhelgaas/src/git-pw/git_pw/api.py", line 169, in patch
    _handle_error('update', exc)
  File "/home/bhelgaas/src/git-pw/git_pw/api.py", line 167, in patch
    rsp.raise_for_status()
  File "/usr/lib/python2.7/dist-packages/requests/models.py", line 940, in raise_for_status
    raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 400 Client Error: Bad Request for url: https://patchwork.kernel.org/api/1.1/patches/11151519/
Bjorn Helgaas Oct. 3, 2019, 9:22 p.m. UTC | #11
[+cc Daniel, Konstantin]

I can't get patchwork delegation via git-pw to work either on ozlabs
or kernel.org.  Any hints on where to look or more data to collect?

Bjorn

On Wed, Sep 25, 2019 at 01:51:20PM -0500, Bjorn Helgaas wrote:
> On Wed, Sep 25, 2019 at 04:33:35PM +0100, Stephen Finucane wrote:
> > On Tue, 2019-09-24 at 14:12 -0500, Bjorn Helgaas wrote:
> > > On Tue, Sep 24, 2019 at 09:45:16AM +0100, Stephen Finucane wrote:
> > > > On Mon, 2019-09-23 at 15:39 -0500, Bjorn Helgaas wrote:
> > > > > On Sat, Sep 21, 2019 at 07:30:46PM +0100, Stephen Finucane wrote:
> > > > > > There have been reports of people being unable to delegate patches to
> > > > > > themselves, despite being a maintainer or the project to which the patch
> > > > > > is associated.
> > > > > > ...
> > > > > I tried the instance at https://patchwork.kernel.org/project/linux-pci/list/
> > > > > to see if it was new enough to work without this fix.  But it also
> > > > > fails, slightly differently:
> > > > > 
> > > > >   $ git config -l | grep "^pw"
> > > > >   pw.server=https://patchwork.kernel.org/api/1.1
> > > > >   pw.project=linux-pci
> > > > >   pw.token=...
> > > > > 
> > > > >   $ git-pw patch update --delegate helgaas 11151519
> > > > >   More than one delegate found: helgaas
> > > > > 
> > > > > Is this another manifestation of the same bug or something else?
> > > > 
> > > > This is a different issue and, unlike the other one, is more feature
> > > > than bug. This is happening because the search for a particular user is
> > > > returning multiple matches. We match on username, first name, last name
> > > > and email, so I imagine you have multiple user accounts on the instance
> > > > and there might be a conflict between an email address of one account
> > > > and a username of another? (Let me know if this isn't the case). The
> > > > easy solution is to use a more specific match. I'd suggest just using
> > > > the email address associated with your user account ([1] suggests this
> > > > is 'bhelgaas@google.com'). We could also support lookup by user ID
> > > > (which would guarantee a single match) but I haven't added that to git-
> > > > pw yet since it didn't seem that usable.
> > > 
> > > Still no workey.  I'm cursed.  The web "Delegate to" dropdown menu
> > > includes "bhelgaas" and my profile seems to be associated with
> > > "bhelgaas" (at least, that's the name in the upper right of the web
> > > page when I'm logged in).
> > > 
> > >   $ git-pw patch update --delegate bhelgaas 11151519
> > >   {u'delegate': [u"User 'bhelgaas' is not a maintainer for project 'Linux PCI development list'"]}
> > >   $ git-pw patch update --delegate bhelgaas@google.com 11151519
> > >   {u'delegate': [u"User 'bhelgaas' is not a maintainer for project 'Linux PCI development list'"]}
> > >   $ git-pw patch update --delegate bjorn@helgaas.com 11151519
> > >   No matching delegates found: bjorn@helgaas.com
> > > 
> > > https://patchwork.kernel.org/user/ also claims I'm a maintainer for
> > > https://patchwork.kernel.org/project/linux-pci/list/ and shows both
> > > bhelgaas@google.com and bjorn@helgaas.com as email addresses.
> > 
> > I'll have a look into this but I've no ideas off the top of my head. If
> > you're comfortable with Python, could you add a couple of print
> > statements to log what we're requesting from the API and what we're
> > getting back and share them here? If not, I'll try look into this next
> > week.
> 
> I am able to change the *state*, e.g.,
> 
>   $ git-pw patch update --state accepted 11151519
>   $ git-pw patch update --state new 11151519
> 
> seem to work fine.
> 
> Not sure if this has enough information to be useful to you, but this
> is the output from:
> 
>   git-pw --debug patch update --delegate bhelgaas@google.com 11151519
> 
> 2019-09-25 13:43:04,838 - git_pw.patch - DEBUG - Updating patch: id=11151519, commit_ref=None, state=None, archived=None
> 2019-09-25 13:43:04,843 - git_pw.config - DEBUG - Retrieved 'server' setting from git-config
> 2019-09-25 13:43:04,843 - git_pw.config - DEBUG - Retrieved 'server' setting from cache
> 2019-09-25 13:43:04,847 - git_pw.config - DEBUG - Retrieved 'project' setting from git-config
> 2019-09-25 13:43:04,847 - git_pw.config - DEBUG - Retrieved 'project' setting from cache
> 2019-09-25 13:43:04,847 - git_pw.config - DEBUG - Retrieved 'project' setting from cache
> 2019-09-25 13:43:04,847 - git_pw.config - DEBUG - Retrieved 'project' setting from cache
> 2019-09-25 13:43:04,847 - git_pw.api - DEBUG - GET https://patchwork.kernel.org/api/1.1/users/
> 2019-09-25 13:43:04,851 - git_pw.config - DEBUG - Retrieved 'token' setting from git-config
> 2019-09-25 13:43:04,852 - git_pw.config - DEBUG - Retrieved 'token' setting from cache
> 2019-09-25 13:43:04,853 - urllib3.connectionpool - DEBUG - Starting new HTTPS connection (1): patchwork.kernel.org:443
> 2019-09-25 13:43:05,414 - urllib3.connectionpool - DEBUG - https://patchwork.kernel.org:443 "GET /api/1.1/users/?q=bhelgaas%40google.com&project=linux-pci HTTP/1.1" 200 167
> 2019-09-25 13:43:05,415 - git_pw.api - DEBUG - Got response
> 2019-09-25 13:43:05,416 - git_pw.config - DEBUG - Retrieved 'server' setting from cache
> 2019-09-25 13:43:05,416 - git_pw.config - DEBUG - Retrieved 'server' setting from cache
> 2019-09-25 13:43:05,416 - git_pw.api - DEBUG - PATCH https://patchwork.kernel.org/api/1.1/patches/11151519/, data=[('delegate', 13257)]
> 2019-09-25 13:43:05,416 - git_pw.config - DEBUG - Retrieved 'token' setting from cache
> 2019-09-25 13:43:05,416 - git_pw.config - DEBUG - Retrieved 'token' setting from cache
> 2019-09-25 13:43:05,417 - urllib3.connectionpool - DEBUG - Starting new HTTPS connection (1): patchwork.kernel.org:443
> 2019-09-25 13:43:05,926 - urllib3.connectionpool - DEBUG - https://patchwork.kernel.org:443 "PATCH /api/1.1/patches/11151519/ HTTP/1.1" 400 93
> 2019-09-25 13:43:05,927 - git_pw.api - ERROR - JSON response
> 2019-09-25 13:43:05,928 - git_pw.api - ERROR - {u'delegate': [u"User 'bhelgaas' is not a maintainer for project 'Linux PCI development list'"]}
> 2019-09-25 13:43:05,928 - git_pw.config - DEBUG - Retrieved 'debug' setting from cache
> Traceback (most recent call last):
>   File "/home/bhelgaas/bin/git-pw", line 10, in <module>
>     sys.exit(cli())
>   File "/usr/lib/python2.7/dist-packages/click/core.py", line 764, in __call__
>     return self.main(*args, **kwargs)
>   File "/usr/lib/python2.7/dist-packages/click/core.py", line 717, in main
>     rv = self.invoke(ctx)
>   File "/usr/lib/python2.7/dist-packages/click/core.py", line 1137, in invoke
>     return _process_result(sub_ctx.command.invoke(sub_ctx))
>   File "/usr/lib/python2.7/dist-packages/click/core.py", line 1137, in invoke
>     return _process_result(sub_ctx.command.invoke(sub_ctx))
>   File "/usr/lib/python2.7/dist-packages/click/core.py", line 956, in invoke
>     return ctx.invoke(self.callback, **ctx.params)
>   File "/usr/lib/python2.7/dist-packages/click/core.py", line 555, in invoke
>     return callback(*args, **kwargs)
>   File "/home/bhelgaas/src/git-pw/git_pw/patch.py", line 180, in update_cmd
>     patch = api.update('patches', patch_id, data)
>   File "/home/bhelgaas/src/git-pw/git_pw/api.py", line 277, in update
>     return patch(url, data).json()
>   File "/home/bhelgaas/src/git-pw/git_pw/api.py", line 169, in patch
>     _handle_error('update', exc)
>   File "/home/bhelgaas/src/git-pw/git_pw/api.py", line 167, in patch
>     rsp.raise_for_status()
>   File "/usr/lib/python2.7/dist-packages/requests/models.py", line 940, in raise_for_status
>     raise HTTPError(http_error_msg, response=self)
> requests.exceptions.HTTPError: 400 Client Error: Bad Request for url: https://patchwork.kernel.org/api/1.1/patches/11151519/
Bjorn Helgaas Oct. 9, 2019, 4:58 p.m. UTC | #12
On Thu, Oct 3, 2019 at 4:22 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> I can't get patchwork delegation via git-pw to work either on ozlabs
> or kernel.org.  Any hints on where to look or more data to collect?

This magically started working on patchwork.kernel.org.  I don't know
what changed, but thank you to whoever fixed it!

> On Wed, Sep 25, 2019 at 01:51:20PM -0500, Bjorn Helgaas wrote:
> > On Wed, Sep 25, 2019 at 04:33:35PM +0100, Stephen Finucane wrote:
> > > On Tue, 2019-09-24 at 14:12 -0500, Bjorn Helgaas wrote:
> > > > On Tue, Sep 24, 2019 at 09:45:16AM +0100, Stephen Finucane wrote:
> > > > > On Mon, 2019-09-23 at 15:39 -0500, Bjorn Helgaas wrote:
> > > > > > On Sat, Sep 21, 2019 at 07:30:46PM +0100, Stephen Finucane wrote:
> > > > > > > There have been reports of people being unable to delegate patches to
> > > > > > > themselves, despite being a maintainer or the project to which the patch
> > > > > > > is associated.
> > > > > > > ...
> > > > > > I tried the instance at https://patchwork.kernel.org/project/linux-pci/list/
> > > > > > to see if it was new enough to work without this fix.  But it also
> > > > > > fails, slightly differently:
> > > > > >
> > > > > >   $ git config -l | grep "^pw"
> > > > > >   pw.server=https://patchwork.kernel.org/api/1.1
> > > > > >   pw.project=linux-pci
> > > > > >   pw.token=...
> > > > > >
> > > > > >   $ git-pw patch update --delegate helgaas 11151519
> > > > > >   More than one delegate found: helgaas
> > > > > >
> > > > > > Is this another manifestation of the same bug or something else?
> > > > >
> > > > > This is a different issue and, unlike the other one, is more feature
> > > > > than bug. This is happening because the search for a particular user is
> > > > > returning multiple matches. We match on username, first name, last name
> > > > > and email, so I imagine you have multiple user accounts on the instance
> > > > > and there might be a conflict between an email address of one account
> > > > > and a username of another? (Let me know if this isn't the case). The
> > > > > easy solution is to use a more specific match. I'd suggest just using
> > > > > the email address associated with your user account ([1] suggests this
> > > > > is 'bhelgaas@google.com'). We could also support lookup by user ID
> > > > > (which would guarantee a single match) but I haven't added that to git-
> > > > > pw yet since it didn't seem that usable.
> > > >
> > > > Still no workey.  I'm cursed.  The web "Delegate to" dropdown menu
> > > > includes "bhelgaas" and my profile seems to be associated with
> > > > "bhelgaas" (at least, that's the name in the upper right of the web
> > > > page when I'm logged in).
> > > >
> > > >   $ git-pw patch update --delegate bhelgaas 11151519
> > > >   {u'delegate': [u"User 'bhelgaas' is not a maintainer for project 'Linux PCI development list'"]}
> > > >   $ git-pw patch update --delegate bhelgaas@google.com 11151519
> > > >   {u'delegate': [u"User 'bhelgaas' is not a maintainer for project 'Linux PCI development list'"]}
> > > >   $ git-pw patch update --delegate bjorn@helgaas.com 11151519
> > > >   No matching delegates found: bjorn@helgaas.com
> > > >
> > > > https://patchwork.kernel.org/user/ also claims I'm a maintainer for
> > > > https://patchwork.kernel.org/project/linux-pci/list/ and shows both
> > > > bhelgaas@google.com and bjorn@helgaas.com as email addresses.
> > >
> > > I'll have a look into this but I've no ideas off the top of my head. If
> > > you're comfortable with Python, could you add a couple of print
> > > statements to log what we're requesting from the API and what we're
> > > getting back and share them here? If not, I'll try look into this next
> > > week.
> >
> > I am able to change the *state*, e.g.,
> >
> >   $ git-pw patch update --state accepted 11151519
> >   $ git-pw patch update --state new 11151519
> >
> > seem to work fine.
> >
> > Not sure if this has enough information to be useful to you, but this
> > is the output from:
> >
> >   git-pw --debug patch update --delegate bhelgaas@google.com 11151519
> >
> > 2019-09-25 13:43:04,838 - git_pw.patch - DEBUG - Updating patch: id=11151519, commit_ref=None, state=None, archived=None
> > 2019-09-25 13:43:04,843 - git_pw.config - DEBUG - Retrieved 'server' setting from git-config
> > 2019-09-25 13:43:04,843 - git_pw.config - DEBUG - Retrieved 'server' setting from cache
> > 2019-09-25 13:43:04,847 - git_pw.config - DEBUG - Retrieved 'project' setting from git-config
> > 2019-09-25 13:43:04,847 - git_pw.config - DEBUG - Retrieved 'project' setting from cache
> > 2019-09-25 13:43:04,847 - git_pw.config - DEBUG - Retrieved 'project' setting from cache
> > 2019-09-25 13:43:04,847 - git_pw.config - DEBUG - Retrieved 'project' setting from cache
> > 2019-09-25 13:43:04,847 - git_pw.api - DEBUG - GET https://patchwork.kernel.org/api/1.1/users/
> > 2019-09-25 13:43:04,851 - git_pw.config - DEBUG - Retrieved 'token' setting from git-config
> > 2019-09-25 13:43:04,852 - git_pw.config - DEBUG - Retrieved 'token' setting from cache
> > 2019-09-25 13:43:04,853 - urllib3.connectionpool - DEBUG - Starting new HTTPS connection (1): patchwork.kernel.org:443
> > 2019-09-25 13:43:05,414 - urllib3.connectionpool - DEBUG - https://patchwork.kernel.org:443 "GET /api/1.1/users/?q=bhelgaas%40google.com&project=linux-pci HTTP/1.1" 200 167
> > 2019-09-25 13:43:05,415 - git_pw.api - DEBUG - Got response
> > 2019-09-25 13:43:05,416 - git_pw.config - DEBUG - Retrieved 'server' setting from cache
> > 2019-09-25 13:43:05,416 - git_pw.config - DEBUG - Retrieved 'server' setting from cache
> > 2019-09-25 13:43:05,416 - git_pw.api - DEBUG - PATCH https://patchwork.kernel.org/api/1.1/patches/11151519/, data=[('delegate', 13257)]
> > 2019-09-25 13:43:05,416 - git_pw.config - DEBUG - Retrieved 'token' setting from cache
> > 2019-09-25 13:43:05,416 - git_pw.config - DEBUG - Retrieved 'token' setting from cache
> > 2019-09-25 13:43:05,417 - urllib3.connectionpool - DEBUG - Starting new HTTPS connection (1): patchwork.kernel.org:443
> > 2019-09-25 13:43:05,926 - urllib3.connectionpool - DEBUG - https://patchwork.kernel.org:443 "PATCH /api/1.1/patches/11151519/ HTTP/1.1" 400 93
> > 2019-09-25 13:43:05,927 - git_pw.api - ERROR - JSON response
> > 2019-09-25 13:43:05,928 - git_pw.api - ERROR - {u'delegate': [u"User 'bhelgaas' is not a maintainer for project 'Linux PCI development list'"]}
> > 2019-09-25 13:43:05,928 - git_pw.config - DEBUG - Retrieved 'debug' setting from cache
> > Traceback (most recent call last):
> >   File "/home/bhelgaas/bin/git-pw", line 10, in <module>
> >     sys.exit(cli())
> >   File "/usr/lib/python2.7/dist-packages/click/core.py", line 764, in __call__
> >     return self.main(*args, **kwargs)
> >   File "/usr/lib/python2.7/dist-packages/click/core.py", line 717, in main
> >     rv = self.invoke(ctx)
> >   File "/usr/lib/python2.7/dist-packages/click/core.py", line 1137, in invoke
> >     return _process_result(sub_ctx.command.invoke(sub_ctx))
> >   File "/usr/lib/python2.7/dist-packages/click/core.py", line 1137, in invoke
> >     return _process_result(sub_ctx.command.invoke(sub_ctx))
> >   File "/usr/lib/python2.7/dist-packages/click/core.py", line 956, in invoke
> >     return ctx.invoke(self.callback, **ctx.params)
> >   File "/usr/lib/python2.7/dist-packages/click/core.py", line 555, in invoke
> >     return callback(*args, **kwargs)
> >   File "/home/bhelgaas/src/git-pw/git_pw/patch.py", line 180, in update_cmd
> >     patch = api.update('patches', patch_id, data)
> >   File "/home/bhelgaas/src/git-pw/git_pw/api.py", line 277, in update
> >     return patch(url, data).json()
> >   File "/home/bhelgaas/src/git-pw/git_pw/api.py", line 169, in patch
> >     _handle_error('update', exc)
> >   File "/home/bhelgaas/src/git-pw/git_pw/api.py", line 167, in patch
> >     rsp.raise_for_status()
> >   File "/usr/lib/python2.7/dist-packages/requests/models.py", line 940, in raise_for_status
> >     raise HTTPError(http_error_msg, response=self)
> > requests.exceptions.HTTPError: 400 Client Error: Bad Request for url: https://patchwork.kernel.org/api/1.1/patches/11151519/
> _______________________________________________
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
Konstantin Ryabitsev Oct. 9, 2019, 4:59 p.m. UTC | #13
On Wed, 9 Oct 2019 at 12:58, Bjorn Helgaas <bjorn.helgaas@gmail.com> wrote:
>
> On Thu, Oct 3, 2019 at 4:22 PM Bjorn Helgaas <helgaas@kernel.org> wrote:
> >
> > I can't get patchwork delegation via git-pw to work either on ozlabs
> > or kernel.org.  Any hints on where to look or more data to collect?
>
> This magically started working on patchwork.kernel.org.  I don't know
> what changed, but thank you to whoever fixed it!

The 2.1.5 upgrade was the magical change. :)

-K
diff mbox series

Patch

diff --git a/patchwork/api/patch.py b/patchwork/api/patch.py
index c9360308..d1c9904d 100644
--- a/patchwork/api/patch.py
+++ b/patchwork/api/patch.py
@@ -105,8 +105,8 @@  class PatchListSerializer(BaseHyperlinkedModelSerializer):
         if not value:
             return value
 
-        if not self.instance.project.maintainer_project.filter(
-                id=value.id).exists():
+        if not value.profile.maintainer_projects.only('id').filter(
+                id=self.instance.project.id).exists():
             raise ValidationError("User '%s' is not a maintainer for project "
                                   "'%s'" % (value, self.instance.project))
         return value
diff --git a/patchwork/tests/api/test_patch.py b/patchwork/tests/api/test_patch.py
index 82ae0184..edae9851 100644
--- a/patchwork/tests/api/test_patch.py
+++ b/patchwork/tests/api/test_patch.py
@@ -284,6 +284,30 @@  class TestPatchAPI(utils.APITestCase):
         self.assertContains(resp, 'Expected one of: %s.' % state.name,
                             status_code=status.HTTP_400_BAD_REQUEST)
 
+    def test_update_legacy_delegate(self):
+        """Regression test for bug #313."""
+        project = create_project()
+        state = create_state()
+        patch = create_patch(project=project, state=state)
+        user_a = create_maintainer(project)
+
+        # create a user (User), then delete the associated UserProfile and save
+        # the user to ensure a new profile is generated
+        user_b = create_user()
+        self.assertEqual(user_b.id, user_b.profile.id)
+        user_b.profile.delete()
+        user_b.save()
+        user_b.profile.maintainer_projects.add(project)
+        user_b.profile.save()
+        self.assertNotEqual(user_b.id, user_b.profile.id)
+
+        self.client.force_authenticate(user=user_a)
+        resp = self.client.patch(self.api_url(patch.id),
+                                 {'delegate': user_b.id})
+        self.assertEqual(status.HTTP_200_OK, resp.status_code, resp)
+        self.assertEqual(Patch.objects.get(id=patch.id).state, state)
+        self.assertEqual(Patch.objects.get(id=patch.id).delegate, user_b)
+
     def test_update_invalid_delegate(self):
         """Update patch with invalid fields.