WebServices function to tell us whether or not a user can set a flag

NEW
Unassigned

Status

()

Bugzilla
WebService
P5
enhancement
9 years ago
3 years ago

People

(Reporter: Kevin Benton, Unassigned)

Tracking

Details

Attachments

(1 attachment)

(Reporter)

Description

9 years ago
User Story:

Mylyn needs to be able to use an API to determine if the logged-in user has the ability to set/update flags before Mylyn submits bug updates so that it knows what the user should be allowed to do.  This will make it easier for Mylyn to process permissions without needing to parse output from show_bug.cgi in hopes of being able to determine current permissions.

Updated

9 years ago
Keywords: APIchange

Comment 1

9 years ago
  For now it would be better to just add a field to config.cgi that says whether or not the currently-logged-in user can set or update a flag, I think, no?
Priority: -- → P5
Summary: RFE: Webservices API to enablement → WebServices function to tell us whether or not a user can set a flag

Comment 2

9 years ago
(In reply to comment #1)
>   For now it would be better to just add a field to config.cgi that says
> whether or not the currently-logged-in user can set or update a flag, I think,
> no?
In cases where the configuration is cached (eclipse.org) this won't work. I realize this is an edge case. I believe Frank's patch on bug#473220 is all that is required (for both group and flag permissions). Frank can you confirm?

Comment 3

9 years ago
(In reply to comment #2)
> (In reply to comment #1)
> >   For now it would be better to just add a field to config.cgi that says
> > whether or not the currently-logged-in user can set or update a flag, I think,
> > no?
> In cases where the configuration is cached (eclipse.org) this won't work. I
> realize this is an edge case. I believe Frank's patch on bug#473220 is all that
> is required (for both group and flag permissions). Frank can you confirm?
Rob, yes if we have the patch for bug#473220 we have the needed  information.

But when I see it right a cached configuration give us a problem. See bug#473201 comment 5.
The patch from bug#473201 is also involved to get flag permissions.

I create bug#473200 and 473201 to get the needed Information for Mylyn but did no changes in Mylyn to verify that this work.

Comment 4

6 years ago
Created attachment 641003 [details] [diff] [review]
patch - v1
Assignee: webservice → koosha.khajeh
Status: NEW → ASSIGNED
Attachment #641003 - Flags: review?(dkl)

Updated

6 years ago
Target Milestone: --- → Bugzilla 4.4
Comment on attachment 641003 [details] [diff] [review]
patch - v1

Review of attachment 641003 [details] [diff] [review]:
-----------------------------------------------------------------

I am not sure this belongs in User.pm for WebServices. I realize it is in the core User.pm already
but for the design of WebServices, I feel this is a better fit in Flag.pm such as Flag.can_set for
example. Any other opinions?

dkl

::: Bugzilla/WebService/User.pm
@@ +240,5 @@
> +
> +    my $flag_type;
> +    if ($params->{name}) {
> +        $flag_type = Bugzilla::FlagType->check($params->{name});
> +    }

It is possible to have multiple flags in Bugzilla all with the same name. For example, a review (bug) flag and a review (attachment) flag. It is possible to have multiple flags of the same type with the same name. FlagType->check() will always just return the first one that is returned by the generated SQL on the backend. So in most cases, we will not be able to use a 'name' param for Flag webservices like we can for other fields. 

The client will need to first get the flag data for the flag they are interested in using something like Flag.get, etc. and then pass in the type_id value to this method.

This is why $user->can_set_flag() takes a FlagType object.
Attachment #641003 - Flags: review?(dkl) → review-

Comment 6

6 years ago
(In reply to David Lawrence [:dkl] from comment #5)
> Comment on attachment 641003 [details] [diff] [review]
> patch - v1
> 
> Review of attachment 641003 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> I am not sure this belongs in User.pm for WebServices. I realize it is in
> the core User.pm already
> but for the design of WebServices, I feel this is a better fit in Flag.pm
> such as Flag.can_set for
> example. Any other opinions?
> 
> dkl

Undoubtedly, it would be a great idea to have a complete set of classes for the web service. But, we need to add a new class (and so much boring lines of POD) for the methods that do not exist before and do not belong to the existing classes. So, this would take us more time and effort.

But, that's what I'm here for. :-)
I would like to work more on web service implementation provided you guys support me actively.
(In reply to Koosha Khajeh Moogahi [:koosha] from comment #6)
> Undoubtedly, it would be a great idea to have a complete set of classes for
> the web service. But, we need to add a new class (and so much boring lines
> of POD) for the methods that do not exist before and do not belong to the
> existing classes. So, this would take us more time and effort.
> 
> But, that's what I'm here for. :-)
> I would like to work more on web service implementation provided you guys
> support me actively.

And I (and we) appreciate and welcome the time and resources you can dedicate to enhancing the webservices code of Bugzilla as it has been sorely lacking for quite some time.

That being said as the WebServices module owner for Bugzilla, I just want us to think through thoroughly any decisions on how we lay out the code now as we will have to maintain it for some time to come. Once we put export methods for clients to use it is not easy to make changes later once they come to rely on it working a specific way. We have a mix of stable and experimental webservice methods now so we have some room to change on the ones not marked stable. But they all need to be stable at some point and I want them all to be consistent and easy for people to discover the ones they need.

We already have a bug under review (on my review list) that creates the new Flag.pm
class and adds in the beginnings of all of the boring POD for us. I just need to get that reviewed and have it committed. Then we can add Flag.can_set on top of that with your changes. bug 596438

dkl
(In reply to David Lawrence [:dkl] from comment #7)
> We already have a bug under review (on my review list) that creates the new
> Flag.pm
> class and adds in the beginnings of all of the boring POD for us. I just
> need to get that reviewed and have it committed. Then we can add
> Flag.can_set on top of that with your changes. bug 596438

Interestingly, now that I begin to review the patch on bug 596438, it already includes a can_set_flag and can_request_flag attribute on the flag data that is returned. So this could be used possibly instead of adding a new can_set/can_request method.

dkl

Comment 9

6 years ago
I'm sure you're making a big mistake about the bug id! bug 596438 is completely unrelated and old. Resolved in 2010!
(In reply to Koosha Khajeh Moogahi [:koosha] from comment #9)
> I'm sure you're making a big mistake about the bug id! bug 596438 is
> completely unrelated and old. Resolved in 2010!

Ugh. sorry. Need more coffee :) It is bug 506538.

https://bugzilla.mozilla.org/show_bug.cgi?id=506538

dkl

Updated

6 years ago
Depends on: 506538

Comment 11

5 years ago
Too late for 4.4. We already released 4.4rc1, and 4.4rc2 is just around the corner. We don't have a plan for a 3rd release candidate, so this bug is postponed to 5.0.
Target Milestone: Bugzilla 4.4 → Bugzilla 5.0

Updated

5 years ago
Assignee: koosha.khajeh → webservice
Status: ASSIGNED → NEW

Updated

3 years ago
Target Milestone: Bugzilla 5.0 → ---
You need to log in before you can comment on or make changes to this bug.