<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Jul 2, 2018 at 12:32 PM Fam Zheng <<a href="mailto:famz@redhat.com">famz@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Fri, 06/29 19:35, Shubham Jain wrote:<br>
> 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.<br>
> ---<br>
>  api/rest.py          |  3 ++-<br>
>  patchew-cli          | 18 +++++++++++++-----<br>
>  tests/test_import.py |  8 ++++----<br>
>  3 files changed, 19 insertions(+), 10 deletions(-)<br>
> <br>
> diff --git a/api/rest.py b/api/rest.py<br>
> index 9b47f37..876be75 100644<br>
> --- a/api/rest.py<br>
> +++ b/api/rest.py<br>
> @@ -423,7 +423,8 @@ class MessagesViewSet(BaseMessageViewSet):<br>
>      serializer_class = MessageSerializer<br>
>      permission_classes = (permissions.IsAuthenticatedOrReadOnly,)<br>
>      parser_classes = (JSONParser, MessagePlainTextParser, )<br>
> -    <br>
> +    authentication_classes = (CsrfExemptSessionAuthentication, )<br>
> +<br>
>      def create(self, request, *args, **kwargs):<br>
>          m = MboxMessage(request.data['mbox'])<br>
>          projects = [p for p in Project.objects.all() if p.recognizes(m)]<br>
> diff --git a/patchew-cli b/patchew-cli<br>
> index f90f3c5..3796caf 100755<br>
> --- a/patchew-cli<br>
> +++ b/patchew-cli<br>
> @@ -220,11 +220,18 @@ class ImportCommand(SubCommand):<br>
>                      print("[OLD] " + mo["Subject"])<br>
>                      return<br>
>              print("[NEW] " + mo["Subject"])<br>
> -            r = self.api_do("import", mboxes=[mo.as_string()])<br>
> -            for p in r:<br>
> -                if p not in projects:<br>
> -                    projects.add(p)<br>
> -                    print(p)<br>
> +            for mbox in [mo.as_string()]:<br>
> +                r = self.rest_api_do(url_cmd="messages",<br>
> +                                     request_method='post',<br>
> +                                     content_type='message/rfc822',<br>
> +                                     data=mbox)<br>
> +                projects_list = [x['resource_uri'].split("messages")[0] for x in r['results']]<br>
> +                for p in projects_list:<br>
> +                    if p not in projects:<br>
> +                        projects.add(p)<br>
> +                        print(p)<br>
> +                if len(projects_list)==0:<br>
> +                    print("The message was not imported to any project. Perhaps you're not logged in as an importer or maintainer")<br>
<br>
Let's report error or raise exception instead of only showing a message if not<br>
logged in as importer/maintainer/suerpuser.<br></blockquote><div>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?</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
>              if ff:<br>
>                  open(ff, "wb").close()<br>
>  <br>
> @@ -251,6 +258,7 @@ class ImportCommand(SubCommand):<br>
>              try:<br>
>                  import_one(f)<br>
>              except:<br>
> +                raise<br>
<br>
Move the print line before raise and drop the following lines?<br></blockquote><div>I think we can remove the raise from here. Since we are already returning 1 here.  </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
>                  print("Error in importing:", f)<br>
>                  traceback.print_exc()<br>
>                  r = 1<br>
> diff --git a/tests/test_import.py b/tests/test_import.py<br>
> index 5693d7e..250f391 100755<br>
> --- a/tests/test_import.py<br>
> +++ b/tests/test_import.py<br>
> @@ -107,21 +107,21 @@ class UnprivilegedImportTest(ImportTest):<br>
>  <br>
>      test_import_belong_to_multiple_projects = None<br>
>  <br>
> -    def check_import_should_fail(self):<br>
> -        self.cli_import("0001-simple-patch.mbox.gz", 1)<br>
> +    def check_import_status(self, exit_status):<br>
> +        self.cli_import("0001-simple-patch.mbox.gz", exit_status)<br>
>          a, b = self.check_cli(["search"])<br>
>          self.assertNotIn('[Qemu-devel] [PATCH] quorum: Only compile when supported',<br>
>                           a.splitlines())<br>
>  <br>
>      def test_anonymous_import(self):<br>
>          self.cli_logout()<br>
> -        self.check_import_should_fail()<br>
> +        self.check_import_status(exit_status=1)<br>
>  <br>
>      def test_normal_user_import(self):<br>
>          self.cli_logout()<br>
>          self.create_user("someuser", "somepass")<br>
>          self.cli_login("someuser", "somepass")<br>
> -        self.check_import_should_fail()<br>
> +        self.check_import_status(exit_status=0)<br>
>  <br>
>      def test_project_update(self):<br>
>          p = Project.objects.all()[0]<br>
> -- <br>
> 2.15.1 (Apple Git-101)<br>
> <br>
> _______________________________________________<br>
> Patchew-devel mailing list<br>
> <a href="mailto:Patchew-devel@redhat.com" target="_blank">Patchew-devel@redhat.com</a><br>
> <a href="https://www.redhat.com/mailman/listinfo/patchew-devel" rel="noreferrer" target="_blank">https://www.redhat.com/mailman/listinfo/patchew-devel</a><br>
</blockquote></div></div>