[Patchew-devel] [PATCH v2 3/3] Make Import Command use Rest

[Fam Zheng]

On Mon, 07/02 13:11, Shubham Jain wrote:
> On Mon, Jul 2, 2018 at 12:32 PM Fam Zheng <famz at redhat.com> wrote:
> 
> > On Fri, 06/29 19:35, Shubham Jain wrote:
> > > Make patchew-cli's Import user REST apis. Changed the exit status of
> > authenticated user from cli test while importing since in REST every
> > authenticated user can access the import api but messages would be imported
> > only to the projects recognised ie whether it imported by maintainer or
> > importer. Also added warning if message is not imported to any project.
> > > ---
> > >  api/rest.py          |  3 ++-
> > >  patchew-cli          | 18 +++++++++++++-----
> > >  tests/test_import.py |  8 ++++----
> > >  3 files changed, 19 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/api/rest.py b/api/rest.py
> > > index 9b47f37..876be75 100644
> > > --- a/api/rest.py
> > > +++ b/api/rest.py
> > > @@ -423,7 +423,8 @@ class MessagesViewSet(BaseMessageViewSet):
> > >      serializer_class = MessageSerializer
> > >      permission_classes = (permissions.IsAuthenticatedOrReadOnly,)
> > >      parser_classes = (JSONParser, MessagePlainTextParser, )
> > > -
> > > +    authentication_classes = (CsrfExemptSessionAuthentication, )
> > > +
> > >      def create(self, request, *args, **kwargs):
> > >          m = MboxMessage(request.data['mbox'])
> > >          projects = [p for p in Project.objects.all() if p.recognizes(m)]
> > > diff --git a/patchew-cli b/patchew-cli
> > > index f90f3c5..3796caf 100755
> > > --- a/patchew-cli
> > > +++ b/patchew-cli
> > > @@ -220,11 +220,18 @@ class ImportCommand(SubCommand):
> > >                      print("[OLD] " + mo["Subject"])
> > >                      return
> > >              print("[NEW] " + mo["Subject"])
> > > -            r = self.api_do("import", mboxes=[mo.as_string()])
> > > -            for p in r:
> > > -                if p not in projects:
> > > -                    projects.add(p)
> > > -                    print(p)
> > > +            for mbox in [mo.as_string()]:
> > > +                r = self.rest_api_do(url_cmd="messages",
> > > +                                     request_method='post',
> > > +                                     content_type='message/rfc822',
> > > +                                     data=mbox)
> > > +                projects_list = [x['resource_uri'].split("messages")[0]
> > for x in r['results']]
> > > +                for p in projects_list:
> > > +                    if p not in projects:
> > > +                        projects.add(p)
> > > +                        print(p)
> > > +                if len(projects_list)==0:
> > > +                    print("The message was not imported to any project.
> > Perhaps you're not logged in as an importer or maintainer")
> >
> > Let's report error or raise exception instead of only showing a message if
> > not
> > logged in as importer/maintainer/suerpuser.
> >
> Raising an exception will return exit status 1, also the condition of
> checking is only just checking the length of projects list. What if an
> importer is logged in and messages not imported to any project, ie there
> are no recognised project. In that case it should return 0 as exit status
> right?

Yes. It should basically work like this:

have permission  |   message format  |     number of     |    result
  to import?     |      is valid?    | imported projects |
---------------------------------------------------------------------
       N                    X                    X             error
       Y                    N                    X             error
       Y                    Y                    X             ok

(X = don't care)

Fam