BC-8569 - Handover of Owner Role

[muratmerdoglu-dp]

Short Description

Links to Ticket and related Pull-Requests

• https://ticketsystem.dbildungscloud.de/browse/BC-8569

Changes

Data-security

Deployment

New Repos, NPM packages or vendor scripts

Screenshots of UI changes

Checklist before merging

• QA: In addition to review, the code has been manually tested (if manual testing is possible)
• PO: Any deviation from requirements was agreed with Product-Owner / ticket author / support-team
• DEV: Every new component is implemented having accessibility in mind (e.g. aria-label, role property)

Notice: Please keep this Pull-Request as a Draft (or add WIP label), until it is ready to be reviewed

[NFriedo]

What do you think about renaming the "swapOwnershipInState" to "setRoomOwner"?

And maybe we can extract duplicate lines(213-219) into a function:

	const setRoomOwner = (userId: string) => {
		const currentOwner = roomMembers.value.find(
			(member) => member.roomRoleName === RoleName.Roomowner
		);
		const memberToBeOwner = roomMembers.value.find(
			(member) => member.userId === userId
		);

		if (!currentOwner || !memberToBeOwner) {
			return;
		}

		updateMemberRole(memberToBeOwner, RoleName.Roomowner);
		updateMemberRole(currentOwner, RoleName.Roomadmin);
	};

	const updateMemberRole = (member: RoomMember, roleName: RoleName) => {
		member.roomRoleName = roleName;
		member.displayRoomRole = roomRole[roleName];
		member.isSelectable = false;
	};

[hoeppner-dataport]

Good idea - I'd recommend to also add throwing an exception

if (!currentOwner || !memberToBeOwner) { 
   throw new Error('setting new owner failed');
}

[muratmerdoglu-dp]

I think throwing any Error is redirected to the error page / or logs the error!

• What about calling fetchMembers() to get fresh members' data?
• or calling showFailure() notification?

[NFriedo]

We could think about renaming 'isHandOverMode" to something like "isOwnerHandOverMode" to make it clearer.

[hoeppner-dataport]

longer but maybe something like "inOwnershipHandoverMode"

[NFriedo]

It's confusing ro me that we use the label from the VRadio and then use an extra <label>. That causes e.g. that the screenreader user just reads out the label from the VRadio.

We should use the label slot from the VRadio like :

        <v-radio>
          <template #label>
            // first key t('pages.rooms.members.roomPermissions.viewer')
           // second key  t("pages.rooms.members.roleChange.Roomviewer.label"
           // in this slot we can e.g. spans to style them as needed :)
          </template>
        </v-radio>

So the Screenreader will read out the whole label and its clear what the label is.

[NFriedo]

My suggestion would be:

<v-radio
  id="roleChangeViewer"
  :value="RoleName.Roomviewer"
  color="primary"
  class="align-start mb-2"
>
  <template #label>
    <div class="d-flex flex-column mt-2">
      {{ t("pages.rooms.members.roomPermissions.viewer") }}
      <span class="radio-label">
        {{ t("pages.rooms.members.roleChange.Roomviewer.label") }}
      </span>
    </div>
  </template>
</v-radio>

So we can also avoid negative margins like "mt-n2" and double labels.

We could even think about using v-for for the radio buttons, to avoid code duplication:
In the template:

<v-radio
  v-for="roomRole in roomRoles"
  :key="roomRole.role"
  :value="roomRole.role"
  color="primary"
  class="align-start mb-2"
>
  <template #label>
    <div class="d-flex flex-column mt-2">
      {{ roomRole.labelHeader }}
      <span
        v-for="labelDescription in roomRole.labelDescriptions"
        :key="labelDescription"
        class="radio-label"
      >
        {{ t(labelDescription) }}
      </span>
    </div>
  </template>
</v-radio>

in the script (maybe with a better var naming e.g. radioOptions)

const roomRoles = computed(() => {
	const baseRoles = [
		{
			role: RoleName.Roomviewer,
			labelHeader: t("pages.rooms.members.roomPermissions.viewer"),
			labelDescriptions: ["pages.rooms.members.roleChange.Roomviewer.label"],
		},
		{
			role: RoleName.Roomeditor,
			labelHeader: t("pages.rooms.members.roomPermissions.editor"),
			labelDescriptions: ["pages.rooms.members.roleChange.Roomeditor.label"],
		},
		{
			role: RoleName.Roomadmin,
			labelHeader: t("pages.rooms.members.roomPermissions.admin"),
			labelDescriptions: ["pages.rooms.members.roleChange.Roomadmin.label"],
		},
	];

	if (isChangeRoleOptionVisible.value) {
		baseRoles.push({
			role: RoleName.Roomowner,
			labelHeader: t("pages.rooms.members.roomPermissions.owner"),
			labelDescriptions: [
				"pages.rooms.members.roleChange.Roomowner.label",
				"pages.rooms.members.roleChange.Roomowner.label.subText",
			],
		});
	}

	return baseRoles;
});

[NFriedo]

Fine for now, like discussed this will be changed in BC-8549 to use defineModel

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
94.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud