Feature/bulk assignment #21

petrKavulok · 2023-02-26T13:50:02Z

related MR TeskaLabs/seacat-auth#167, TeskaLabs/seacat-auth#179

Screen.Recording.2023-04-20.at.16.14.24.mov

Screen.Recording.2023-04-20.at.16.18.46.mov

…checkboxes

…init

petrKavulok · 2023-06-01T13:16:05Z

Screen.Recording.2023-06-01.at.15.09.15.1.mp4

Pe5h4

@petrKavulok It looks good, however I have some minor change requests

Pe5h4 · 2023-06-02T07:25:08Z

public/locales/cs/translation.json

+		"Add global role(s)": "Přidat globální role",
+		"Global roles have to be individually specified": "Globální role musí být specifikovány",
+		"Do you really want to leave this screen? Selected data will be lost": "Přesunem na jinou stránku budou ztracena zvolená data.",
+		"More":"Více ...",


@petrKavulok Missing space between the colon and translation

Pe5h4 · 2023-06-02T07:30:50Z

src/modules/auth/components/RoleDropdown.js

+		try {
+			response = await SeaCatAuthAPI.get(`/role/${id}`, {params: parameters});
+			let selectedTenantsCopy = [...selectedTenants];
+			let ten = {...objCopy, roles: response.data};


@petrKavulok use a better naming of ten variable, I suppose its a tenantsRolesObject or something

Pe5h4 · 2023-06-02T07:33:51Z

src/modules/auth/components/RoleDropdown.js

+				msg = t("BulkAssignmentContainer|If you wish to assign/unassign global roles, select individual roles. Otherwise leave it as is");
+			}
+		} else if (tenantObj?.selectedRole && (tenantObj.selectedRole.length > 0)) {
+			msg = t("BulkAssignmentContainer|Tenant") + " '" + tenantObj._id + "' " + t("BulkAssignmentContainer|and selected roles will be assigned to/unassigned from selected credentials");


@petrKavulok Can you wrap that the dynamic string to {t("BulkAssignmentContainer|Tenant")} '${ tenantObj._id}' ${t("BulkAssignmentContainer|and selected roles will be assigned to/unassigned from selected credentials")}

Pe5h4 · 2023-06-02T07:34:01Z

src/modules/auth/components/RoleDropdown.js

+		} else if (tenantObj?.selectedRole && (tenantObj.selectedRole.length > 0)) {
+			msg = t("BulkAssignmentContainer|Tenant") + " '" + tenantObj._id + "' " + t("BulkAssignmentContainer|and selected roles will be assigned to/unassigned from selected credentials");
+		} else {
+			msg = t("BulkAssignmentContainer|Selected credentials will be assigned to/removed from tenant") + " '" + tenantObj._id + "'";


@petrKavulok Same here

Pe5h4 · 2023-06-02T07:47:22Z

src/modules/auth/credentials/BulkAssignmentContainer.js

+
+	/* Actual modified data (this data set was compared compared with `selectedCredentials` and `assigned: true` was added to matching credentials. Assigned:true property
+	disables the "+"" button) to be displayed in Credentials list data table */
+	const datatableCredentialsData = useMemo(() => {


@petrKavulok Since you have duplicit code in datatableCredentialsData and datatableTenantsData, I propose to unify the guts of it:

create a new method, which will return desired result:

const matchAssigned = (data, selectedEvent) => { let tableData = []; if (data) { data.map((dataObj) => { let matchedObj = selectedEvent.find(obj => obj._id === dataObj._id); if (matchedObj) { matchedObj['assigned'] = true; tableData.push(matchedObj); } else { dataObj['assigned'] = false; tableData.push(dataObj); } }) } return tableData; }

and use it within the memoised function

const datatableCredentialsData = useMemo(() => { return matchAssigned(data, selectedCredentials); }, [data, selectedCredentials]);

and repeat for datatableTenantsData

Always think about your code - dont repeat yourself or simplify what can be simplified or done more performant :)

Pe5h4 · 2023-06-02T07:48:02Z

src/modules/auth/credentials/BulkAssignmentContainer.js

+		return credentialsTableData
+	}, [data, selectedCredentials]);
+
+	const datatableTenantsData = useMemo(() => {


@petrKavulok as my comment above

Pe5h4 · 2023-06-02T07:48:45Z

src/modules/auth/credentials/BulkAssignmentContainer.js

+		Unless we'd like to do bulk *unassignment*, then unassigning from tenant as a whole requires { tenant1: "UNASSIGN-TENANT", tenant2: [role1, role2,..], ...} */
+		// let globalRoles = [];
+		selectedTenants.map((obj) => {
+			console.log('obj: ', obj);


@petrKavulok Forgotten console.log

Pe5h4 · 2023-06-02T07:49:08Z

src/modules/auth/credentials/BulkAssignmentContainer.js

+		/* adjustments to data structure. selectedTenants is an array of objects. Server expects credential_ids to be an object with
+		tenants as keys and array of roles as their values { tenant1: [role1, role2], tenant2: [], ...}
+		Unless we'd like to do bulk *unassignment*, then unassigning from tenant as a whole requires { tenant1: "UNASSIGN-TENANT", tenant2: [role1, role2,..], ...} */
+		// let globalRoles = [];


@petrKavulok Why is this variable commented?

Pe5h4 · 2023-06-02T07:53:09Z

src/modules/auth/credentials/BulkAssignmentContainer.js

+			} else if (actionType === '/tenant_unassign_many') {
+				roles = "UNASSIGN-TENANT";
+			};
+			// here in code, we are using `{_id: Global roles, ...}` which represents global roles, but the api call expects `_id: '*'` as a representation of global roles


@petrKavulok Try to write comments more object based - meaning - its always better not to use any personification, when writing comments about the code - so, not we, I, ... but rather writ about what it does and why it is used. eg.

// This part of the code uses {_id: Global roles, ...}, which is the representation of global roles, however the API call expects _idto be defined as asterisk'*' as a representation of global roles

Pe5h4 · 2023-06-02T07:54:26Z

src/modules/auth/credentials/BulkAssignmentContainer.js

+	};
+
+	const unselectGlobalRole = (roleIndex) => {
+		// [{global: false, selectedRole: []}]


@petrKavulok Remove or add any comment to it

Pe5h4

@petrKavulok Please, use proper constants for styling

Pe5h4 · 2023-06-02T08:01:49Z

src/modules/auth/credentials/credentials.scss

+}
+
+.credentials-suspended {
+	color: #73818f!important;


@petrKavulok why??? we have $secondary color for that. also you have wrong indentation here

Pe5h4 · 2023-06-02T08:02:16Z

src/modules/auth/credentials/credentials.scss

+
+@media (max-width: 994px) {
+    .bulk-actions-wraper {
+        // display: grid;


@petrKavulok If not needed, remove it

Pe5h4 · 2023-06-02T08:04:05Z

src/modules/auth/credentials/credentials.scss

+}
+
+.selected-row:hover {
+    background-color: var(--table-bg-color-hover);


@petrKavulok $table-bg-color-hover -- you can load it from table styles (@import "~asab-webui/styles/components/table.scss";) or you can modify ASAB WebUI index.scss file in styles/constants/ and import this (and maybe some other) color in there

Pe5h4 · 2023-06-02T08:04:46Z

src/modules/auth/credentials/credentials.scss

+
+.selected-row:hover {
+    background-color: var(--table-bg-color-hover);
+    color: var(--text-color);


@petrKavulok $text-color

Pe5h4 · 2023-06-02T08:07:53Z

src/modules/auth/credentials/credentials.scss

+		}
+	}
+    & .data-table-tbody {
+        background-color: var(--bg-color) ;


@petrKavulok $bg-color

Pe5h4

@petrKavulok Ok, this looks good. Please read my comment and deliberately update, if you will find it useful

Pe5h4 · 2023-06-06T14:40:19Z

src/modules/auth/credentials/BulkAssignmentContainer.js

+			// This part of the code uses {_id: Global roles, ...}, which is the representation of global roles, however the API call expects _id to be defined as an asterisk '*' as a representation of global roles
+			tenantObj[obj._id] = roles;
+		})
+		globalRoles[0].global ? tenantObj['*'] = globalRoles[0].selectedRole : null;


@petrKavulok Well, for the sake of readability, I would advise you to rewrite the code to:

if(globalRoles[0].global) { tenantObj['*'] = globalRoles[0].selectedRole }

I dont say your expression is wrong, but I would personally avoid writing ternary operators where not necessary (on the other hand, in conditional rendering you would have to use ternary operators).

…n func

petrKavulok · 2023-06-07T13:26:30Z

Ahoj @Pe5h4 ,

I refactored the ternary operator. ✔️
While testing, I found a bug in the tooltip message, which was showing the same message for globalRoles and the first item in selectedTenants (they were using the same id for tooltip). It's fixed now.

I did not encounter any other issues while testing.

petrKavulok added 6 commits February 26, 2023 21:25

CredentialsTenntsAssignContainer init

80e1f17

added route for CredentialsTennatnsAssignContainer

ec0b084

Tenants card init. fetchAllTenants function and display in form with …

f5de62b

…checkboxes

retrieve user info function

42964a0

indentation using tabs

9c73773

credentials card initiation

ba0c034

petrKavulok self-assigned this Feb 26, 2023

petrKavulok added 23 commits February 26, 2023 21:54

retrieve assigned tenants and retrieveCredentialsForDropdown funtion …

df88022

…init

grid styling for cards

a230386

add Credentials dropdown with filtering

3938c6e

dropdown filtering

3433c01

alerts text update, added check for response result !== 'OK'

2edc192

obsolete code removal, syntax update

b9df088

translation.json files update

6703a43

remove credentials from list functionality

b96ef00

locales updates

1da919c

submit function init

d1fc1fd

Merge branch 'main' into feature/bulk-assignment

1684656

renaming container

408d3ab

bulk assignment router route renaming

25a7f2b

css grid for bulk assignment styling declaration

e3a7763

bulk actions button in credentials list datatable's header

6684237

bulk assignment container optimization

cf269cb

unifying cards and tables styling in for bulk assignment containers

923e81e

translations comments and code polishment

e4407ae

locales - duplicate code removal

e16fba2

small-screen layout for bulk assignment

bdfaa0e

Merge branch 'main' into feature/bulk-assignment

97a9771

Merge branch 'main' into feature/bulk-assignment

4a12472

styling update for BulkAssignmentContainer

f0d12b0

petrKavulok added 8 commits May 31, 2023 17:32

unify dropdown to be used with both global and non global roles

56cc7c6

separate global roles from tenants

7cb3dff

asab-webui submodule udpaate

4ed80d4

CHANGELOG.md update

61f0ec3

obsolete, performance draining code removal

ae399eb

suspended credentials color change with classname

9d64863

suspended credentials color change with classname

ae36c5d

button disabling conditions update

1db8afd

Merge branch 'main' into feature/bulk-assignment

1fc7827

petrKavulok requested a review from Pe5h4 June 1, 2023 13:17

Pe5h4 requested changes Jun 2, 2023

View reviewed changes

typo update

a4cc52a

Pe5h4 requested changes Jun 2, 2023

View reviewed changes

petrKavulok added 9 commits June 2, 2023 10:14

variable naming update

6046b51

tooltip message dynamic string update

f1814a7

tooltip message dynamic string update

520e041

removing duplicate code

d4ea318

forgotten console log removal

061def6

obsolete code removal, comments update

29754c2

color variable adjustments

518538c

obsolete code removal

8f1994f

CHANGELOG.md update

767552b

petrKavulok requested a review from Pe5h4 June 6, 2023 11:16

Pe5h4 approved these changes Jun 6, 2023

View reviewed changes

petrKavulok added 2 commits June 7, 2023 15:10

global roles tooltip message bugfix

3cc8657

if statement instead of ternary opertor for global roles in BulkActio…

0d7c404

…n func

petrKavulok merged commit b2a9fb2 into main Jun 8, 2023

petrKavulok deleted the feature/bulk-assignment branch June 8, 2023 11:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/bulk assignment #21

Feature/bulk assignment #21

petrKavulok commented Feb 26, 2023 •

edited

Loading

petrKavulok commented Jun 1, 2023

Pe5h4 left a comment

Pe5h4 Jun 2, 2023

Pe5h4 Jun 2, 2023

Pe5h4 Jun 2, 2023

Pe5h4 Jun 2, 2023

Pe5h4 Jun 2, 2023

Pe5h4 Jun 2, 2023

Pe5h4 Jun 2, 2023

Pe5h4 Jun 2, 2023

Pe5h4 Jun 2, 2023

Pe5h4 Jun 2, 2023

Pe5h4 left a comment

Pe5h4 Jun 2, 2023

Pe5h4 Jun 2, 2023

Pe5h4 Jun 2, 2023

Pe5h4 Jun 2, 2023

Pe5h4 Jun 2, 2023

Pe5h4 left a comment

Pe5h4 Jun 6, 2023

petrKavulok commented Jun 7, 2023

Feature/bulk assignment #21

Feature/bulk assignment #21

Conversation

petrKavulok commented Feb 26, 2023 • edited Loading

petrKavulok commented Jun 1, 2023

Pe5h4 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

Pe5h4 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

Pe5h4 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

petrKavulok commented Jun 7, 2023

petrKavulok commented Feb 26, 2023 •

edited

Loading