Feature/rename workspace/47 #72

zak39 · 2021-05-31T07:48:42Z

Hey @StCyr 👋

The renaming is (almost) finished but the sidebar menu (AppNavigation) doesn't update at the same time.

Give you some advice me ? ^^'

…'GE-'/'U-' groups only

…ral homepage when reloading pages Signed-off-by: Cyrille Bollu <cyr.debian@bollu.be>

Signed-off-by: Cyrille Bollu <cyr.debian@bollu.be>

it matching other more specific routes. Signed-off-by: Cyrille Bollu <cyr.debian@bollu.be>

Signed-off-by: Cyrille Bollu <cyr.debian@bollu.be>

…ckend Signed-off-by: Cyrille Bollu <cyr.debian@bollu.be>

Signed-off-by: Cyrille Bollu <cyr.debian@bollu.be>

…'GE-'/'U-' groups only

StCyr

Hello Baptiste,

I've requested some changes

StCyr · 2021-05-31T08:41:51Z

src/SpaceDetails.vue

-						{{ t('workspace', 'Rename space') }}
-					</ActionButton>
+						@submit="renameSpace">
+						{{ t('workspace', 'Rename spaceAAAAA') }}


some old garbage

I corrected it !

StCyr · 2021-05-31T08:42:46Z

src/SpaceDetails.vue

 			// TODO
+			const oldSpaceName = this.$root.$data.spaces[this.$route.params.space].name


better write it:

const oldSpaceName = this.$route.params.space

I will test it !

Okay I took it into account

StCyr · 2021-05-31T08:45:34Z

src/SpaceDetails.vue

+					const data = resp.data
+
+					if (data.statuscode === 204) {
+						this.$root.$data.spaces[data.space] = this.$root.$data.spaces[oldSpaceName]


Should be:

let space = this.$root.$data.spaces[oldSpaceName] space.name = data.space Vue.set(this.$root.$data.spaces, data.space, space) delete this.$root.$data.spaces[oldSpaceName]

Aaah okay ! Instead of replacing the space you advise me to create a new space in spaces list

I took it into account !

StCyr · 2021-05-31T08:46:45Z

src/SpaceDetails.vue

+					}
+				})
+				.catch((e) => {
+					Vue.set(this.$root.$data.spaces[this.$route.params.space], 'name', oldSpaceName)


Okay I understand what you mean !

And I will correct it

…ivate' else we don't have spaces list

…a/workspace into feature/rename-workspace/47

StCyr

Please review my remarks.

There are also issues in the SpaceDetails.vue file.

StCyr · 2021-06-02T20:16:13Z

appinfo/routes.php

@@ -29,6 +29,11 @@
 			'url' => '/spaces',
 			'verb' => 'GET'
 		],
+		[
+			'name' => 'workspace#create',
+			'url' => '/spaces',


Could you change the url to /api/spaces?

I think it's better to do it like this.

I'll change the existing routes one of these days

StCyr · 2021-06-02T20:16:40Z

appinfo/routes.php

@@ -49,6 +54,11 @@
 			'url' => '/group/{gid}/users',
 			'verb' => 'GET'
 		],
+		[
+			'name' => 'workspace#rename',
+			'url' => '/spaces/{folderId}',


Could you change the url to /api/spaces/{$folderId}?

I think it's better to do it like this.

I'll change the existing routes one of these days

StCyr · 2021-06-02T20:16:58Z

appinfo/routes.php

@@ -59,5 +69,10 @@
 			'requirements' => array('path' => '.+'),
 			'defaults' => array('path' => 'dummy'),
 		],
+		[
+			'name' => 'workspace#rename',


This route is already defined above

Ouch... I am correcting it !

StCyr · 2021-06-02T20:18:50Z

lib/Controller/WorkspaceController.php

@@ -67,12 +79,12 @@ public function __construct(
     * may use.
     *
     * @NoAdminRequired
-     * 
+     * @NoCSRFRequired


@NoCSRTRequired isn't needed?!?

Ouch... I had to add it to test when I didn't success to get all users :/

I tested the route with curl

…duplicated

zak39 · 2021-06-03T12:44:57Z

@StCyr for Routes : If we want to add /api/<ressource>. It has to formalize our responses. In this way, we will have structure responses

What do you think ?

For example to support my proposition : When I create a space I return this json :

{
  "space": "Galadriel",
  "id_space": 213,
  "groups": {
    "GE-Galadriel": 31,
    "U-Galadriel": 31
  },
  "space_advanced_permissions": true,
  "space_assign_groups": [
    "GE-Galadriel",
    "U-Galadriel"
  ],
  "acl": {
    "state": true,
    "group_manage": "GE-Galadriel"
  },
  "statuscode": 201
}

I don't know if this structure is better. But, to know if the resource is created, we can filter as it : .statuscode.

…rks Back-end only

zak39 · 2021-06-10T09:18:04Z

@StCyr I resolved conflicts but I need your help ! 😕

Can you check again this PR, please ? 🙂

StCyr

Hello,
I've reviewed your PR.
I've a remark related to access-control on your 'rename' method + have found some regression removing the '/index.php' part in your axios calls

lib/Controller/WorkspaceController.php

StCyr · 2021-06-10T13:15:36Z

lib/Service/GroupfolderService.php

+     */
+    public function get($folderId) {
+        $response = $this->httpClient->get(
+            $this->urlGenerator->getBaseUrl() . '/apps/groupfolders/folders/' . $folderId,


add /index.php

StCyr · 2021-06-10T13:15:47Z

lib/Service/GroupfolderService.php

-            $this->urlGenerator->getBaseUrl() . '/index.php/apps/groupfolders/folders/' . $id . '/groups/' . $gid ,
+    public function delete($id) {
+        $response = $this->httpClient->delete(
+            $this->urlGenerator->getBaseUrl() . '/apps/groupfolders/folders/' . $id,


add /index.php

It's okay 👌

StCyr · 2021-06-10T13:15:55Z

lib/Service/GroupfolderService.php

-            $this->urlGenerator->getBaseUrl() . '/index.php/apps/groupfolders/folders/' . $id,
+    public function rename($folderId, $newSpaceName) {
+        $response = $this->httpClient->post(
+            $this->urlGenerator->getBaseUrl() . '/apps/groupfolders/folders/'. $folderId .'/mountpoint',


add /index.php

It's okay 👌

StCyr · 2021-06-10T13:16:01Z

lib/Service/GroupfolderService.php

+     */
+    public function attachGroup($folderId, $gid) {
+        $response = $this->httpClient->post(
+            $this->urlGenerator->getBaseUrl() . '/apps/groupfolders/folders/' . $folderId . '/groups',


add /index.php

It's okay 👌

… space to rename

…ce.php

zak39 · 2021-06-11T15:50:39Z

It's okay @StCyr now ;)

…x.php/'

zak39 and others added 26 commits May 28, 2021 17:03

feat(Controller, Vue, Route): It's possible to rename one space with …

1c30a31

…'GE-'/'U-' groups only

feat(Controller): Take subgroups into account in the renaming

7fa8bb7

Merge branch 'main' into feature/rename-workspace/47

252c055

fix(Class): Change ->APP_ID to self::APP_ID because APP_ID is a constant

94aa3ab

Adds a 'catch-all' php route to prevent being redirected to NC's gene…

31d45aa

…ral homepage when reloading pages Signed-off-by: Cyrille Bollu <cyr.debian@bollu.be>

Removes the 'npm test' action as we don't have such tests yet

7a1c88e

Signed-off-by: Cyrille Bollu <cyr.debian@bollu.be>

Fixes the tests that broke after creation of the UserService service

edc7583

Signed-off-by: Cyrille Bollu <cyr.debian@bollu.be>

Moves all PHP constants to a dedicated file

dc4e518

Signed-off-by: Cyrille Bollu <cyr.debian@bollu.be>

Adds package-lock.json to repository (needed for github CI)

4bfba22

Signed-off-by: Cyrille Bollu <cyr.debian@bollu.be>

Removes javascript tests from github CI as we don't have tests yet

64fbd52

Signed-off-by: Cyrille Bollu <cyr.debian@bollu.be>

Adds a logger to WorkspaceController

e221338

Signed-off-by: Cyrille Bollu <cyr.debian@bollu.be>

Moves the default PHP route at the end of the routes.php file to prevent

142b90d

it matching other more specific routes. Signed-off-by: Cyrille Bollu <cyr.debian@bollu.be>

Sorts user alphabeticaly in UserTable component

4f37212

Signed-off-by: Cyrille Bollu <cyr.debian@bollu.be>

Removes duplicate 'use' statement in WorkspaceAccessControlMiddleware

9208a1d

Signed-off-by: Cyrille Bollu <cyr.debian@bollu.be>

Fixes use '$this->APP_ID' and not just 'APP_ID' in Application.php

4e06e04

Signed-off-by: Cyrille Bollu <cyr.debian@bollu.be>

Formats users properly when returning a workspace to the frontend

8bbadc3

Signed-off-by: Cyrille Bollu <cyr.debian@bollu.be>

Updates frontend to understand new format of users returned by the ba…

c4602b5

…ckend Signed-off-by: Cyrille Bollu <cyr.debian@bollu.be>

Adds avatar in the UserTable component

80c8960

Signed-off-by: Cyrille Bollu <cyr.debian@bollu.be>

Gets a workspace 'subgroups that a user is member of

2e3cee0

Signed-off-by: Cyrille Bollu <cyr.debian@bollu.be>

Adapts frontend for the new format of the users returned by the backend

60bd6f4

Signed-off-by: Cyrille Bollu <cyr.debian@bollu.be>

Displays a user's groups in the UserTable

1ca6bdb

Signed-off-by: Cyrille Bollu <cyr.debian@bollu.be>

Fixes UserTable component not working when showing group's users

ff277d5

Signed-off-by: Cyrille Bollu <cyr.debian@bollu.be>

Moves the formatUser() function to UserService.php

fb2baa1

Signed-off-by: Cyrille Bollu <cyr.debian@bollu.be>

feat(Controller, Vue, Route): It's possible to rename one space with …

ed1fc1b

…'GE-'/'U-' groups only

feat(Controller): Take subgroups into account in the renaming

bb9cfa0

fix(Class): Change ->APP_ID to self::APP_ID because APP_ID is a constant

e64d4b1

StCyr requested changes May 31, 2021

View reviewed changes

zak39 added 3 commits June 1, 2021 15:58

fix(Controller): The formatUser method should be 'public' and not 'pr…

030e5ed

…ivate' else we don't have spaces list

Conflict(After Pull): Change files after a git pull

787f854

refactor(Controller, Service): The 'rename' method is cleaner

410d54f

Merge branch 'feature/rename-workspace/47' of https://github.com/araw…

656e012

…a/workspace into feature/rename-workspace/47

StCyr requested changes Jun 2, 2021

View reviewed changes

zak39 added 2 commits June 3, 2021 14:29

fix(route): Deleting the '/spaces/folderId - verb: PATCH' because it …

16277ff

…duplicated

fix(Controller): Deleting '@NoCSRFRequired' which was used as a test

2096a0e

zak39 mentioned this pull request Jun 3, 2021

Should we structure our APIs ? #87

Open

zak39 added 2 commits June 4, 2021 16:22

fix(Vue): Update the method's parameter to rename a space name. It wo…

21ac223

…rks Back-end only

fix(Vue): It is adapted for Vuex

fed83d4

zak39 self-assigned this Jun 8, 2021

Merge branch 'main' into feature/rename-workspace/47

396add8

StCyr requested changes Jun 10, 2021

View reviewed changes

zak39 and others added 5 commits June 10, 2021 16:28

fix(Vue): Deleting spaces trigged by no-trailing-spaces when built it

e869eb3

fix(Vuex): Adding mutations and actions to add and delete in spaces list

17f876c

feature(Vue, Controller): Check if the current user is manager of the…

960ef07

… space to rename

refactor(Service): Add 'index.php/' a few methods in GroupfolderServi…

42adab0

…ce.php

Merge branch 'main' into feature/rename-workspace/47

d1dd284

zak39 and others added 9 commits June 14, 2021 09:56

Merge branch 'main' into feature/rename-workspace/47

5211dc3

fix(JavaScript): Solving the problems related to CI and missing 'inde…

02dee81

…x.php/'

Merge branch 'main' into feature/rename-workspace/47

6f2a766

Merge branch 'main' into feature/rename-workspace/47

8603828

fix(JavaScript): Solving the problems related to CI and missing 'inde…

0d6c246

…x.php/'

Fix(Vue, Javascript, Css, PHP): Fixed after solving conflicts

dc5366e

fix(Vue): Remove 'index.php' in API's URLs

04ec958

Merge branch 'main' into feature/rename-workspace/47

c9e3e35

fix(Javascrip): Fixed double 'removeSpace' methods

8e8faf6

StCyr merged commit 78c82d6 into main Jun 15, 2021

LivOriona linked an issue Jun 15, 2021 that may be closed by this pull request

Be able to rename a workspace as a general manager #47

Closed

zak39 deleted the feature/rename-workspace/47 branch July 13, 2021 13:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/rename workspace/47 #72

Feature/rename workspace/47 #72

zak39 commented May 31, 2021

StCyr left a comment

StCyr May 31, 2021

zak39 Jun 2, 2021

StCyr May 31, 2021

zak39 Jun 2, 2021

zak39 Jun 2, 2021

StCyr May 31, 2021

zak39 Jun 2, 2021

zak39 Jun 2, 2021

StCyr May 31, 2021

zak39 Jun 2, 2021

StCyr left a comment

StCyr Jun 2, 2021

StCyr Jun 2, 2021

StCyr Jun 2, 2021

zak39 Jun 3, 2021

StCyr Jun 2, 2021

zak39 Jun 3, 2021

zak39 Jun 3, 2021

zak39 commented Jun 3, 2021

zak39 commented Jun 10, 2021

StCyr left a comment

StCyr Jun 10, 2021

zak39 Jun 11, 2021

StCyr Jun 10, 2021

zak39 Jun 11, 2021

StCyr Jun 10, 2021

zak39 Jun 11, 2021

StCyr Jun 10, 2021

zak39 Jun 11, 2021

zak39 commented Jun 11, 2021

		// TODO
		const oldSpaceName = this.$root.$data.spaces[this.$route.params.space].name

Feature/rename workspace/47 #72

Feature/rename workspace/47 #72

Conversation

zak39 commented May 31, 2021

StCyr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StCyr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zak39 commented Jun 3, 2021

zak39 commented Jun 10, 2021

StCyr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zak39 commented Jun 11, 2021