You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by "vttranlina (via GitHub)" <gi...@apache.org> on 2023/02/28 10:28:35 UTC
[GitHub] [james-project] vttranlina opened a new pull request, #1471: JAMES-3893 Add a WebAdmin API allowing creation/updation user identity
vttranlina opened a new pull request, #1471:
URL: https://github.com/apache/james-project/pull/1471
Jira: https://issues.apache.org/jira/browse/JAMES-3893
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] Arsnael commented on a diff in pull request #1471: JAMES-3893 Add a WebAdmin API allowing creation/updation user identity
Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1471:
URL: https://github.com/apache/james-project/pull/1471#discussion_r1122898278
##########
server/protocols/webadmin/webadmin-jmap/src/test/java/org/apache/james/webadmin/data/jmap/UserIdentitiesRoutesTest.java:
##########
@@ -290,4 +292,219 @@ void listIdentitiesShouldReturnNotFoundWhenCanNotQueryDefaultIdentity() {
" \"details\": null" +
"}");
}
+
+ @Test
+ void createIdentityShouldWork() {
+ String creationRequest = "" +
+ " {" +
+ " \"name\": \"create name 1\"," +
+ " \"email\": \"bob@domain.tld\"," +
+ " \"mayDelete\": true," +
Review Comment:
Actually good point, in the spec it is supposed to be server-set only indeed.
Just remove this in the test, and maybe make sure with an other test that if a request comes with mayDelete to false (and even true?), we should reject it, as for server-set values, in the spec, it says the client MUST NOT send this property when creating a new object of this type
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] vttranlina commented on a diff in pull request #1471: JAMES-3893 Add a WebAdmin API allowing creation/updation user identity
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on code in PR #1471:
URL: https://github.com/apache/james-project/pull/1471#discussion_r1122843606
##########
server/protocols/webadmin/webadmin-jmap/src/main/java/org/apache/james/webadmin/data/jmap/dto/UserIdentity.java:
##########
@@ -113,6 +131,43 @@ public UserIdentity(String name, String email, String id,
this.replyTo = replyTo;
}
+ @JsonCreator
+ public UserIdentity(@JsonProperty("name") String name,
Review Comment:
> why there is no mayDelete field
The current API does not accept input `mayDelete` field
Quote:
```
org.apache.james.jmap.api.identity.IdentityRepository#save
mayDelete alway is TRUE
```
Should we modify it for web-admin case?
> I would see more this as a UserIdentityCreate or something similar
Yes, it will be clear if we have a separate object `UserIdentityCreate` / `UserIdentiteUpdate`,
But, IMO, it is simple pojo, we can re-use it in a single object `UserIdentity`, less boilerplate.
So, I didn't create UserIdentityCreate...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] vttranlina commented on pull request #1471: JAMES-3893 Add a WebAdmin API allowing creation/updation user identity
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on PR #1471:
URL: https://github.com/apache/james-project/pull/1471#issuecomment-1453019712
Rebase master & squash old commit & fixup latest comment
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa merged pull request #1471: JAMES-3893 Add a WebAdmin API allowing creation/updation user identity
Posted by "chibenwa (via GitHub)" <gi...@apache.org>.
chibenwa merged PR #1471:
URL: https://github.com/apache/james-project/pull/1471
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] Arsnael commented on a diff in pull request #1471: JAMES-3893 Add a WebAdmin API allowing creation/updation user identity
Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1471:
URL: https://github.com/apache/james-project/pull/1471#discussion_r1122900563
##########
server/protocols/webadmin/webadmin-jmap/src/main/java/org/apache/james/webadmin/data/jmap/dto/UserIdentity.java:
##########
@@ -113,6 +131,43 @@ public UserIdentity(String name, String email, String id,
this.replyTo = replyTo;
}
+ @JsonCreator
+ public UserIdentity(@JsonProperty("name") String name,
Review Comment:
Forget the mayDelete field remark, as said in latest comment it's indeed server-set.
> Yes, it will be clear if we have a separate object UserIdentityCreate / UserIdentiteUpdate,
> But, IMO, it is simple pojo, we can re-use it in a single object UserIdentity, less boilerplate.
Honestly it was a bit confusing to me, I had to re-read it to understand what it was for^^'
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] Arsnael commented on a diff in pull request #1471: JAMES-3893 Add a WebAdmin API allowing creation/updation user identity
Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1471:
URL: https://github.com/apache/james-project/pull/1471#discussion_r1122898278
##########
server/protocols/webadmin/webadmin-jmap/src/test/java/org/apache/james/webadmin/data/jmap/UserIdentitiesRoutesTest.java:
##########
@@ -290,4 +292,219 @@ void listIdentitiesShouldReturnNotFoundWhenCanNotQueryDefaultIdentity() {
" \"details\": null" +
"}");
}
+
+ @Test
+ void createIdentityShouldWork() {
+ String creationRequest = "" +
+ " {" +
+ " \"name\": \"create name 1\"," +
+ " \"email\": \"bob@domain.tld\"," +
+ " \"mayDelete\": true," +
Review Comment:
Actually good point, in the spec it is supposed to be server-set only indeed.
Just remove this in the test, and maybe make sure with an other test that if a request comes with mayDelete to false, we should reject it, as for server-set values, in the spec, it says the client MUST NOT send this property when creating a new object of this type
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] Arsnael commented on pull request #1471: JAMES-3893 Add a WebAdmin API allowing creation/updation user identity
Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on PR #1471:
URL: https://github.com/apache/james-project/pull/1471#issuecomment-1452856378
Can rebase as well :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] Arsnael commented on a diff in pull request #1471: JAMES-3893 Add a WebAdmin API allowing creation/updation user identity
Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1471:
URL: https://github.com/apache/james-project/pull/1471#discussion_r1123953689
##########
server/apps/distributed-app/docs/modules/ROOT/pages/operate/webadmin.adoc:
##########
@@ -718,9 +718,9 @@ In order to do that: `default=true`
The web-admin server will return `404` response code when the default identity can not be found.
-=== Creating a user identity
+=== Creating a JMAP user-set identity
Review Comment:
Take out the user-set in the doc, just keep user (so other lines as well). It makes no sense in the doc
##########
src/site/markdown/server/manage-webadmin.md:
##########
@@ -523,9 +523,9 @@ Valid status includes:
- `FAILED`: Error encountered while executing this step. Check the logs.
- `ABORTED`: Won't be executed because of previous step failures.
-### Creating a user identity
+### Creating a JMAP user-set identity
Review Comment:
idem
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] quantranhong1999 commented on a diff in pull request #1471: JAMES-3893 Add a WebAdmin API allowing creation/updation user identity
Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on code in PR #1471:
URL: https://github.com/apache/james-project/pull/1471#discussion_r1122864422
##########
server/protocols/webadmin/webadmin-jmap/src/test/java/org/apache/james/webadmin/data/jmap/UserIdentitiesRoutesTest.java:
##########
@@ -290,4 +292,219 @@ void listIdentitiesShouldReturnNotFoundWhenCanNotQueryDefaultIdentity() {
" \"details\": null" +
"}");
}
+
+ @Test
+ void createIdentityShouldWork() {
+ String creationRequest = "" +
+ " {" +
+ " \"name\": \"create name 1\"," +
+ " \"email\": \"bob@domain.tld\"," +
+ " \"mayDelete\": true," +
Review Comment:
`mayDelete` is false -> server-set
`mayDelete` is true -> user-set
instead?
Here I think we only be able to create/update user-set identity
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] vttranlina commented on a diff in pull request #1471: JAMES-3893 Add a WebAdmin API allowing creation/updation user identity
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on code in PR #1471:
URL: https://github.com/apache/james-project/pull/1471#discussion_r1122887723
##########
server/protocols/webadmin/webadmin-jmap/src/test/java/org/apache/james/webadmin/data/jmap/UserIdentitiesRoutesTest.java:
##########
@@ -290,4 +292,219 @@ void listIdentitiesShouldReturnNotFoundWhenCanNotQueryDefaultIdentity() {
" \"details\": null" +
"}");
}
+
+ @Test
+ void createIdentityShouldWork() {
+ String creationRequest = "" +
+ " {" +
+ " \"name\": \"create name 1\"," +
+ " \"email\": \"bob@domain.tld\"," +
+ " \"mayDelete\": true," +
Review Comment:
> mayDelete is false -> server-set
> mayDelete is true -> user-sett
this is what we want or this is currently already?
// Currently, the Jmap method (for user) is also can not set `mayDelete`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] vttranlina commented on a diff in pull request #1471: JAMES-3893 Add a WebAdmin API allowing creation/updation user identity
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on code in PR #1471:
URL: https://github.com/apache/james-project/pull/1471#discussion_r1122936340
##########
server/apps/distributed-app/docs/modules/ROOT/pages/operate/webadmin.adoc:
##########
@@ -718,6 +718,68 @@ In order to do that: `default=true`
The web-admin server will return `404` response code when the default identity can not be found.
+=== Creating a user identity
Review Comment:
I'm not sure,
the POST API is just a create a new entry
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] quantranhong1999 commented on a diff in pull request #1471: JAMES-3893 Add a WebAdmin API allowing creation/updation user identity
Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on code in PR #1471:
URL: https://github.com/apache/james-project/pull/1471#discussion_r1122549856
##########
server/protocols/webadmin/webadmin-jmap/src/main/java/org/apache/james/webadmin/data/jmap/UserIdentityRoutes.java:
##########
@@ -20,42 +20,50 @@
package org.apache.james.webadmin.data.jmap;
import static org.apache.james.webadmin.Constants.SEPARATOR;
+import static spark.Spark.halt;
import java.util.Comparator;
import java.util.List;
import java.util.Optional;
import javax.inject.Inject;
+import javax.mail.internet.AddressException;
import org.apache.james.core.Username;
import org.apache.james.jmap.api.identity.IdentityRepository;
import org.apache.james.util.FunctionalUtils;
import org.apache.james.webadmin.Routes;
import org.apache.james.webadmin.data.jmap.dto.UserIdentity;
import org.apache.james.webadmin.utils.ErrorResponder;
+import org.apache.james.webadmin.utils.JsonExtractException;
+import org.apache.james.webadmin.utils.JsonExtractor;
import org.apache.james.webadmin.utils.JsonTransformer;
import org.apache.james.webadmin.utils.ParametersExtractor;
import org.eclipse.jetty.http.HttpStatus;
import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
import spark.HaltException;
import spark.Request;
import spark.Response;
import spark.Service;
public class UserIdentityRoutes implements Routes {
public static final String USERS = "/users";
- public static final String IDENTITIES = "/identities";
private static final String USER_NAME = ":userName";
+ public static final String USERS_IDENTITY_BASE_PATH = USERS + SEPARATOR + USER_NAME + SEPARATOR + "/identities";
Review Comment:
Just a reminder to remove the `/` out of `/identities` after rebase.
##########
server/apps/distributed-app/docs/modules/ROOT/pages/operate/webadmin.adoc:
##########
@@ -718,6 +718,68 @@ In order to do that: `default=true`
The web-admin server will return `404` response code when the default identity can not be found.
+=== Creating a user identity
Review Comment:
```suggestion
=== Provisioning a JMAP user-set identity
##########
server/apps/distributed-app/docs/modules/ROOT/pages/operate/webadmin.adoc:
##########
@@ -718,6 +718,68 @@ In order to do that: `default=true`
The web-admin server will return `404` response code when the default identity can not be found.
+=== Creating a user identity
+
+API to creating a new identity
Review Comment:
```suggestion
API to create a new JMAP user-set identity
```
Maybe?
The idea here is to distinguish with server-set identity which I guess we can not really create them with this API.
##########
server/apps/distributed-app/docs/modules/ROOT/pages/operate/webadmin.adoc:
##########
@@ -718,6 +718,68 @@ In order to do that: `default=true`
The web-admin server will return `404` response code when the default identity can not be found.
+=== Creating a user identity
+
+API to creating a new identity
+....
+curl -XPOST http://ip:port/users/{username}/identities \
+-d '{
+ "name": "Bob",
+ "email": "bob@domain.tld",
+ "mayDelete": true,
+ "htmlSignature": "a html signature",
+ "textSignature": "a text signature",
+ "bcc": [{
+ "email": "boss2@domain.tld",
+ "name": "My Boss 2"
+ }],
+ "replyTo": [{
+ "email": "boss@domain.tld",
+ "name": "My Boss"
+ }],
+ "sortOrder": 0
+ }' \
+-H "Content-Type: application/json"
+....
+
+Response codes:
+
+* 201: The new identity were successfully created
+* 404: The username is unknown
+* 400: The payload is invalid
+
+Resource name ``username'' represents a valid user
+
+=== Updating a user identity
+
+API to updating an exist identity
Review Comment:
```suggestion
API to update an exist JMAP user-set identity
##########
src/site/markdown/server/manage-webadmin.md:
##########
@@ -523,6 +523,71 @@ Valid status includes:
- `FAILED`: Error encountered while executing this step. Check the logs.
- `ABORTED`: Won't be executed because of previous step failures.
+### Creating a user identity
+
+API to creating a new identity
Review Comment:
idem as above
##########
server/apps/distributed-app/docs/modules/ROOT/pages/operate/webadmin.adoc:
##########
@@ -718,6 +718,68 @@ In order to do that: `default=true`
The web-admin server will return `404` response code when the default identity can not be found.
+=== Creating a user identity
+
+API to creating a new identity
+....
+curl -XPOST http://ip:port/users/{username}/identities \
+-d '{
+ "name": "Bob",
+ "email": "bob@domain.tld",
+ "mayDelete": true,
+ "htmlSignature": "a html signature",
+ "textSignature": "a text signature",
+ "bcc": [{
+ "email": "boss2@domain.tld",
+ "name": "My Boss 2"
+ }],
+ "replyTo": [{
+ "email": "boss@domain.tld",
+ "name": "My Boss"
+ }],
+ "sortOrder": 0
+ }' \
+-H "Content-Type: application/json"
+....
+
+Response codes:
+
+* 201: The new identity were successfully created
+* 404: The username is unknown
+* 400: The payload is invalid
+
+Resource name ``username'' represents a valid user
+
+=== Updating a user identity
Review Comment:
```suggestion
=== Updating a JMAP user-set identity
##########
server/apps/distributed-app/docs/modules/ROOT/pages/operate/webadmin.adoc:
##########
@@ -718,6 +718,68 @@ In order to do that: `default=true`
The web-admin server will return `404` response code when the default identity can not be found.
+=== Creating a user identity
+
+API to creating a new identity
+....
+curl -XPOST http://ip:port/users/{username}/identities \
+-d '{
+ "name": "Bob",
+ "email": "bob@domain.tld",
+ "mayDelete": true,
+ "htmlSignature": "a html signature",
+ "textSignature": "a text signature",
+ "bcc": [{
+ "email": "boss2@domain.tld",
+ "name": "My Boss 2"
+ }],
+ "replyTo": [{
+ "email": "boss@domain.tld",
+ "name": "My Boss"
+ }],
+ "sortOrder": 0
+ }' \
+-H "Content-Type: application/json"
+....
+
+Response codes:
+
+* 201: The new identity were successfully created
Review Comment:
```suggestion
* 201: The new identity was successfully created
```
Or `is` to match the present tense as the below sentences.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] vttranlina commented on a diff in pull request #1471: JAMES-3893 Add a WebAdmin API allowing creation/updation user identity
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on code in PR #1471:
URL: https://github.com/apache/james-project/pull/1471#discussion_r1122852361
##########
server/protocols/webadmin/webadmin-jmap/src/test/java/org/apache/james/webadmin/data/jmap/UserIdentitiesRoutesTest.java:
##########
@@ -290,4 +292,219 @@ void listIdentitiesShouldReturnNotFoundWhenCanNotQueryDefaultIdentity() {
" \"details\": null" +
"}");
}
+
+ @Test
+ void createIdentityShouldWork() {
+ String creationRequest = "" +
+ " {" +
+ " \"name\": \"create name 1\"," +
+ " \"email\": \"bob@domain.tld\"," +
+ " \"mayDelete\": true," +
Review Comment:
My mistake (I copied the payload)
If `mayDelete` is always TRUE (server set), the `mayDelete` property in the payload is dangling
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] quantranhong1999 commented on a diff in pull request #1471: JAMES-3893 Add a WebAdmin API allowing creation/updation user identity
Posted by "quantranhong1999 (via GitHub)" <gi...@apache.org>.
quantranhong1999 commented on code in PR #1471:
URL: https://github.com/apache/james-project/pull/1471#discussion_r1122995699
##########
server/apps/distributed-app/docs/modules/ROOT/pages/operate/webadmin.adoc:
##########
@@ -718,6 +718,68 @@ In order to do that: `default=true`
The web-admin server will return `404` response code when the default identity can not be found.
+=== Creating a user identity
Review Comment:
Just another paraphrase explains that using create api to provision identity for users. Anyway no big deal for this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] Arsnael commented on a diff in pull request #1471: JAMES-3893 Add a WebAdmin API allowing creation/updation user identity
Posted by "Arsnael (via GitHub)" <gi...@apache.org>.
Arsnael commented on code in PR #1471:
URL: https://github.com/apache/james-project/pull/1471#discussion_r1122778649
##########
server/protocols/webadmin/webadmin-jmap/src/main/java/org/apache/james/webadmin/data/jmap/dto/UserIdentity.java:
##########
@@ -113,6 +131,43 @@ public UserIdentity(String name, String email, String id,
this.replyTo = replyTo;
}
+ @JsonCreator
+ public UserIdentity(@JsonProperty("name") String name,
Review Comment:
I don't get:
- why there is no mayDelete field (I think the always true is if it's missing? but if it is filled it should not be server set? Can't the create identity webadmin route be used for creating non default identities as well logically?)
- I don't think it's a valid constructor... I would see more this as a UserIdentityCreate or something similar. The identity does not exist yet, there is no id for example, I would tend to think it is a slightly different object?
##########
server/protocols/webadmin/webadmin-jmap/src/test/java/org/apache/james/webadmin/data/jmap/UserIdentitiesRoutesTest.java:
##########
@@ -290,4 +292,219 @@ void listIdentitiesShouldReturnNotFoundWhenCanNotQueryDefaultIdentity() {
" \"details\": null" +
"}");
}
+
+ @Test
+ void createIdentityShouldWork() {
+ String creationRequest = "" +
+ " {" +
+ " \"name\": \"create name 1\"," +
+ " \"email\": \"bob@domain.tld\"," +
+ " \"mayDelete\": true," +
Review Comment:
that would go with my previous point.. you do declare the mayDelete field in your creation payload?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] vttranlina commented on a diff in pull request #1471: JAMES-3893 Add a WebAdmin API allowing creation/updation user identity
Posted by "vttranlina (via GitHub)" <gi...@apache.org>.
vttranlina commented on code in PR #1471:
URL: https://github.com/apache/james-project/pull/1471#discussion_r1122844797
##########
server/protocols/webadmin/webadmin-jmap/src/main/java/org/apache/james/webadmin/data/jmap/UserIdentityRoutes.java:
##########
@@ -20,42 +20,50 @@
package org.apache.james.webadmin.data.jmap;
import static org.apache.james.webadmin.Constants.SEPARATOR;
+import static spark.Spark.halt;
import java.util.Comparator;
import java.util.List;
import java.util.Optional;
import javax.inject.Inject;
+import javax.mail.internet.AddressException;
import org.apache.james.core.Username;
import org.apache.james.jmap.api.identity.IdentityRepository;
import org.apache.james.util.FunctionalUtils;
import org.apache.james.webadmin.Routes;
import org.apache.james.webadmin.data.jmap.dto.UserIdentity;
import org.apache.james.webadmin.utils.ErrorResponder;
+import org.apache.james.webadmin.utils.JsonExtractException;
+import org.apache.james.webadmin.utils.JsonExtractor;
import org.apache.james.webadmin.utils.JsonTransformer;
import org.apache.james.webadmin.utils.ParametersExtractor;
import org.eclipse.jetty.http.HttpStatus;
import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
import spark.HaltException;
import spark.Request;
import spark.Response;
import spark.Service;
public class UserIdentityRoutes implements Routes {
public static final String USERS = "/users";
- public static final String IDENTITIES = "/identities";
private static final String USER_NAME = ":userName";
+ public static final String USERS_IDENTITY_BASE_PATH = USERS + SEPARATOR + USER_NAME + SEPARATOR + "/identities";
Review Comment:
I will rebase later
It was fixed in https://github.com/apache/james-project/pull/1470
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org