~emersion/hut-dev

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
6 2

[PATCH 1/2] lists: Add acl delete

Details
Message ID
<20220126004901.330479-1-admin@xenrox.net>
DKIM signature
pass
Download raw message
Patch: +55 -0
---
 doc/hut.1.scd                     |  3 +++
 lists.go                          | 31 +++++++++++++++++++++++++++++++
 srht/listssrht/gql.go             | 10 ++++++++++
 srht/listssrht/operations.graphql | 11 +++++++++++
 4 files changed, 55 insertions(+)

diff --git a/doc/hut.1.scd b/doc/hut.1.scd
index 843179f..ae6bc7a 100644
--- a/doc/hut.1.scd
+++ b/doc/hut.1.scd
@@ -316,6 +316,9 @@ Options are:
*acl list* [list]
	List ACL entries of a mailing list.

*acl delete* <ID>
	Delete an ACL entry.

## hg

*list* [owner]
diff --git a/lists.go b/lists.go
index 6a2a3cc..c6ff6f9 100644
--- a/lists.go
+++ b/lists.go
@@ -355,6 +355,7 @@ func newListsACLCommand() *cobra.Command {
		Short: "Manage access-control lists",
	}
	cmd.AddCommand(newListsACLListCommand())
	cmd.AddCommand(newListsACLDeleteCommand())
	return cmd
}

@@ -405,6 +406,36 @@ func newListsACLListCommand() *cobra.Command {
	return cmd
}

func newListsACLDeleteCommand() *cobra.Command {
	run := func(cmd *cobra.Command, args []string) {
		ctx := cmd.Context()
		c := createClient("lists", cmd)

		id, err := parseInt32(args[0])
		if err != nil {
			log.Fatal(err)
		}

		acl, err := listssrht.DeleteACL(c.Client, ctx, id)
		if err != nil {
			log.Fatal(err)
		} else if acl == nil {
			log.Fatalf("failed to delete ACL entry with ID %d", id)
		}

		fmt.Printf("Deleted ACL entry for %q in mailing list %q\n", acl.Entity.CanonicalName, acl.List.Name)
	}

	cmd := &cobra.Command{
		Use:               "delete <ID>",
		Short:             "Delete an ACL entry",
		Args:              cobra.ExactArgs(1),
		ValidArgsFunction: cobra.NoFileCompletions,
		Run:               run,
	}
	return cmd
}

func getMailingListID(c *Client, ctx context.Context, name, owner string) int32 {
	var (
		list *listssrht.MailingList
diff --git a/srht/listssrht/gql.go b/srht/listssrht/gql.go
index 13a2b36..d5710e4 100644
--- a/srht/listssrht/gql.go
+++ b/srht/listssrht/gql.go
@@ -510,3 +510,13 @@ func UpdatePatchset(client *gqlclient.Client, ctx context.Context, id int32, sta
	err = client.Execute(ctx, op, &respData)
	return respData.UpdatePatchset, err
}

func DeleteACL(client *gqlclient.Client, ctx context.Context, id int32) (deleteACL *MailingListACL, err error) {
	op := gqlclient.NewOperation("mutation deleteACL ($id: Int!) {\n\tdeleteACL(id: $id) {\n\t\tentity {\n\t\t\tcanonicalName\n\t\t}\n\t\tlist {\n\t\t\tname\n\t\t}\n\t}\n}\n")
	op.Var("id", id)
	var respData struct {
		DeleteACL *MailingListACL
	}
	err = client.Execute(ctx, op, &respData)
	return respData.DeleteACL, err
}
diff --git a/srht/listssrht/operations.graphql b/srht/listssrht/operations.graphql
index 970930a..e2cba22 100644
--- a/srht/listssrht/operations.graphql
+++ b/srht/listssrht/operations.graphql
@@ -197,3 +197,14 @@ mutation updatePatchset($id: Int!, $status: PatchsetStatus!) {
        subject
    }
}

mutation deleteACL($id: Int!) {
    deleteACL(id: $id) {
        entity {
            canonicalName
        }
        list {
            name
        }
    }
}
-- 
2.35.0

[PATCH 2/2] lists: Add acl update

Details
Message ID
<20220126004901.330479-2-admin@xenrox.net>
In-Reply-To
<20220126004901.330479-1-admin@xenrox.net> (view parent)
DKIM signature
pass
Download raw message
Patch: +150 -0
---
I am not that happy with the usability, --permissions reply,browse,post
feels kinda clunky. Not sure how to improve it though.

 doc/hut.1.scd                     |  8 ++++
 lists.go                          | 66 +++++++++++++++++++++++++++++++
 srht/listssrht/gql.go             | 34 ++++++++++++++++
 srht/listssrht/operations.graphql | 22 +++++++++++
 srht/listssrht/strings.go         | 20 ++++++++++
 5 files changed, 150 insertions(+)

diff --git a/doc/hut.1.scd b/doc/hut.1.scd
index ae6bc7a..eb967b9 100644
--- a/doc/hut.1.scd
+++ b/doc/hut.1.scd
@@ -319,6 +319,14 @@ Options are:
*acl delete* <ID>
	Delete an ACL entry.

*acl update* <user/email address> [options...]
	Update or add an ACL entry.

	Options are:

	*-p*, *--permissions* <strings...>
		List of permissions to set (browse, reply, post, moderate).

## hg

*list* [owner]
diff --git a/lists.go b/lists.go
index c6ff6f9..88ec561 100644
--- a/lists.go
+++ b/lists.go
@@ -356,6 +356,7 @@ func newListsACLCommand() *cobra.Command {
	}
	cmd.AddCommand(newListsACLListCommand())
	cmd.AddCommand(newListsACLDeleteCommand())
	cmd.AddCommand(newListsACLUpdateCommand())
	return cmd
}

@@ -436,6 +437,67 @@ func newListsACLDeleteCommand() *cobra.Command {
	return cmd
}

func newListsACLUpdateCommand() *cobra.Command {
	var permissions []string
	run := func(cmd *cobra.Command, args []string) {
		ctx := cmd.Context()

		input, err := listssrht.ParsePermissions(permissions)
		if err != nil {
			log.Fatal(err)
		}

		isSender := strings.Contains(args[0], "@")
		name, _, instance := getMailingListName(ctx, cmd)
		c := createClientWithInstance("lists", cmd, instance)

		list, err := listssrht.MailingListIDByName(c.Client, ctx, name)
		if err != nil {
			log.Fatal(err)
		} else if list == nil {
			log.Fatalf("no such mailing list %s", name)
		}

		var acl *listssrht.MailingListACL
		if !isSender {
			user, err := listssrht.UserIDByName(c.Client, ctx, args[0])
			if err != nil {
				log.Fatal(err)
			} else if user == nil {
				log.Fatalf("no such user %q", args[0])
			}

			acl, err = listssrht.UpdateUserACL(c.Client, ctx, list.Id, user.Id, input)
			if err != nil {
				log.Fatal(err)
			}
		} else {
			acl, err = listssrht.UpdateSenderACL(c.Client, ctx, list.Id, args[0], input)
			if err != nil {
				log.Fatal(err)
			}
		}

		if acl == nil {
			log.Fatal("failed to update/create ACL entry")
		}

		fmt.Printf("Updated access rights for %q in mailing list %q\n", acl.Entity.CanonicalName, name)
	}

	cmd := &cobra.Command{
		Use:               "update <user/email address>",
		Short:             "Update/add ACL entries",
		Args:              cobra.ExactArgs(1),
		ValidArgsFunction: cobra.NoFileCompletions,
		Run:               run,
	}
	cmd.Flags().StringSliceVarP(&permissions, "permissions", "p", nil, "ACL permissions")
	cmd.RegisterFlagCompletionFunc("permissions", completePermissions)
	cmd.MarkFlagRequired("permissions")
	return cmd
}

func getMailingListID(c *Client, ctx context.Context, name, owner string) int32 {
	var (
		list *listssrht.MailingList
@@ -551,3 +613,7 @@ func completePatchsetID(cmd *cobra.Command, args []string, toComplete string) ([

	return patchList, cobra.ShellCompDirectiveNoFileComp
}

func completePermissions(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
	return []string{"browse", "reply", "post", "moderate"}, cobra.ShellCompDirectiveNoFileComp
}
diff --git a/srht/listssrht/gql.go b/srht/listssrht/gql.go
index d5710e4..cc899ae 100644
--- a/srht/listssrht/gql.go
+++ b/srht/listssrht/gql.go
@@ -480,6 +480,16 @@ func AclByListName(client *gqlclient.Client, ctx context.Context, name string) (
	return respData.MailingListByName, err
}

func UserIDByName(client *gqlclient.Client, ctx context.Context, username string) (userByName *User, err error) {
	op := gqlclient.NewOperation("query userIDByName ($username: String!) {\n\tuserByName(username: $username) {\n\t\tid\n\t}\n}\n")
	op.Var("username", username)
	var respData struct {
		UserByName *User
	}
	err = client.Execute(ctx, op, &respData)
	return respData.UserByName, err
}

func MailingListSubscribe(client *gqlclient.Client, ctx context.Context, listID int32) (mailingListSubscribe *MailingListSubscription, err error) {
	op := gqlclient.NewOperation("mutation mailingListSubscribe ($listID: Int!) {\n\tmailingListSubscribe(listID: $listID) {\n\t\tlist {\n\t\t\tname\n\t\t\towner {\n\t\t\t\tcanonicalName\n\t\t\t}\n\t\t}\n\t}\n}\n")
	op.Var("listID", listID)
@@ -520,3 +530,27 @@ func DeleteACL(client *gqlclient.Client, ctx context.Context, id int32) (deleteA
	err = client.Execute(ctx, op, &respData)
	return respData.DeleteACL, err
}

func UpdateUserACL(client *gqlclient.Client, ctx context.Context, listID int32, userID int32, input ACLInput) (updateUserACL *MailingListACL, err error) {
	op := gqlclient.NewOperation("mutation updateUserACL ($listID: Int!, $userID: Int!, $input: ACLInput!) {\n\tupdateUserACL(listID: $listID, userID: $userID, input: $input) {\n\t\tentity {\n\t\t\tcanonicalName\n\t\t}\n\t}\n}\n")
	op.Var("listID", listID)
	op.Var("userID", userID)
	op.Var("input", input)
	var respData struct {
		UpdateUserACL *MailingListACL
	}
	err = client.Execute(ctx, op, &respData)
	return respData.UpdateUserACL, err
}

func UpdateSenderACL(client *gqlclient.Client, ctx context.Context, listID int32, address string, input ACLInput) (updateSenderACL *MailingListACL, err error) {
	op := gqlclient.NewOperation("mutation updateSenderACL ($listID: Int!, $address: String!, $input: ACLInput!) {\n\tupdateSenderACL(listID: $listID, address: $address, input: $input) {\n\t\tentity {\n\t\t\tcanonicalName\n\t\t}\n\t}\n}\n")
	op.Var("listID", listID)
	op.Var("address", address)
	op.Var("input", input)
	var respData struct {
		UpdateSenderACL *MailingListACL
	}
	err = client.Execute(ctx, op, &respData)
	return respData.UpdateSenderACL, err
}
diff --git a/srht/listssrht/operations.graphql b/srht/listssrht/operations.graphql
index e2cba22..667ca58 100644
--- a/srht/listssrht/operations.graphql
+++ b/srht/listssrht/operations.graphql
@@ -167,6 +167,12 @@ fragment acl on GeneralACL {
    moderate
}

query userIDByName($username: String!) {
    userByName(username: $username) {
        id
    }
}

mutation mailingListSubscribe($listID: Int!) {
    mailingListSubscribe(listID: $listID) {
        list {
@@ -208,3 +214,19 @@ mutation deleteACL($id: Int!) {
        }
    }
}

mutation updateUserACL($listID: Int!, $userID: Int!, $input: ACLInput!) {
    updateUserACL(listID: $listID, userID: $userID, input: $input) {
        entity {
            canonicalName
        }
    }
}

mutation updateSenderACL($listID: Int!, $address: String!, $input: ACLInput!) {
    updateSenderACL(listID: $listID, address: $address, input: $input) {
        entity {
            canonicalName
        }
    }
}
diff --git a/srht/listssrht/strings.go b/srht/listssrht/strings.go
index 1f60958..f6f8ff6 100644
--- a/srht/listssrht/strings.go
+++ b/srht/listssrht/strings.go
@@ -64,3 +64,23 @@ func PermissionIcon(permission bool) string {
	}
	return termfmt.Red.Sprint("✗")
}

func ParsePermissions(permissions []string) (ACLInput, error) {
	var input ACLInput
	for _, permission := range permissions {
		switch strings.ToLower(permission) {
		case "browse":
			input.Browse = true
		case "reply":
			input.Reply = true
		case "post":
			input.Post = true
		case "moderate":
			input.Moderate = true
		default:
			return input, fmt.Errorf("invalid permission: %q", permission)
		}
	}

	return input, nil
}
--
2.35.0
Details
Message ID
<RQLONiJQn8tOYvmEgumqUGmzPzXpP9-3kwiA6_JOYSGbHkQHkOBK7aCdKo7NRAdJ9fwEBOwcQsoPpchTd9J5EP8WaZK9r2SZBZoEKYKnpN4=@emersion.fr>
In-Reply-To
<20220126004901.330479-1-admin@xenrox.net> (view parent)
DKIM signature
pass
Download raw message
Pushed patch 1/2, thanks!

Re: [PATCH 2/2] lists: Add acl update

Details
Message ID
<eX3YA4PtwHYVcDikei-fYD2rdY2W9Tn-cpqNcEPq6IEXG5bTPlmaYyjW3wjTsymG5ab5ByjO7LuiU7yiJpUHTY42dQduZ_e8tWLIAmO6wv8=@emersion.fr>
In-Reply-To
<20220126004901.330479-2-admin@xenrox.net> (view parent)
DKIM signature
pass
Download raw message
On Wednesday, January 26th, 2022 at 01:49, Thorben Günther <admin@xenrox.net> wrote:

> I am not that happy with the usability, --permissions reply,browse,post
> feels kinda clunky. Not sure how to improve it though.

Maybe we could change the syntax to

    acl update <user> <permissions>

where permissions can be

    [+-]perm|perm|…

e.g.

    browse|reply     Set to browse and reply, remove any previous permission
    +reply           Add reply permission
    -post|moderate   Remove post and moderate permissions

Thoughts?

In any case, should be fine to merge a basic version at first, then improve it
later.

Re: [PATCH 2/2] lists: Add acl update

Details
Message ID
<20220208112017.owft4t3itlexk5so@xenrox.net>
In-Reply-To
<eX3YA4PtwHYVcDikei-fYD2rdY2W9Tn-cpqNcEPq6IEXG5bTPlmaYyjW3wjTsymG5ab5ByjO7LuiU7yiJpUHTY42dQduZ_e8tWLIAmO6wv8=@emersion.fr> (view parent)
DKIM signature
pass
Download raw message
On Tue, Feb 08, 2022 at 10:15:27AM +0000, Simon Ser wrote:
>
> Maybe we could change the syntax to
>
>     acl update <user> <permissions>
>
> where permissions can be
>
>     [+-]perm|perm|…
>
> e.g.
>
>     browse|reply     Set to browse and reply, remove any previous permission
>     +reply           Add reply permission
>     -post|moderate   Remove post and moderate permissions

That looks much more usable, but I don't think it will be easy to
generate completions for that (if possible at all).
While it is possible to return different completions depending on the
argument position/length, I am not sure if we can split completions on
something other than a space.

Re: [PATCH 2/2] lists: Add acl update

Details
Message ID
<9E7bNrhj-L6LGtTdA_zkUD78EgSLc4wzDsyVUzo4ob93GjMUKvRsco1-bbuJr-oX5uF2TUrIUG43hd73whRkoXZmiYA9QbDcRwr09wJI9ns=@emersion.fr>
In-Reply-To
<20220208112017.owft4t3itlexk5so@xenrox.net> (view parent)
DKIM signature
pass
Download raw message
On Tuesday, February 8th, 2022 at 12:20, Thorben Günther <admin@xenrox.net> wrote:

> On Tue, Feb 08, 2022 at 10:15:27AM +0000, Simon Ser wrote:
> >
> > Maybe we could change the syntax to
> >
> >     acl update <user> <permissions>
> >
> > where permissions can be
> >
> >     [+-]perm|perm|…
> >
> > e.g.
> >
> >     browse|reply     Set to browse and reply, remove any previous permission
> >     +reply           Add reply permission
> >     -post|moderate   Remove post and moderate permissions
>
> That looks much more usable, but I don't think it will be easy to
> generate completions for that (if possible at all).
> While it is possible to return different completions depending on the
> argument position/length, I am not sure if we can split completions on
> something other than a space.

I think the toComplete arg of the completion callback should contain the
partial ACL, then we can suggest completions depending on that value? Or am I
missing something?

Re: [PATCH 2/2] lists: Add acl update

Details
Message ID
<20220216110101.4kz53wvtwsqwn7h7@xenrox.net>
In-Reply-To
<9E7bNrhj-L6LGtTdA_zkUD78EgSLc4wzDsyVUzo4ob93GjMUKvRsco1-bbuJr-oX5uF2TUrIUG43hd73whRkoXZmiYA9QbDcRwr09wJI9ns=@emersion.fr> (view parent)
DKIM signature
pass
Download raw message
On Tue, Feb 15, 2022 at 06:12:42PM +0000, Simon Ser wrote:
>
> I think the toComplete arg of the completion callback should contain the
> partial ACL, then we can suggest completions depending on that value? Or am I
> missing something?

Yes we should be able to implement it like this. My problem stemmed from
ShellCompDirectiveNoSpace not working - but that seems to be a problem
of my shell setup. Will try to code some kind of mockup to check how
viable this design will be.
In case we end up using it, we should probably use the same idea for
e.g. git acl list, so everything will be more consistent.
Reply to thread Export thread (mbox)