From 954d0a06370ad614fc3193670c36c5c9baae9d08 Mon Sep 17 00:00:00 2001
From: Jannat Patel <pateljannat2308@gmail.com>
Date: Thu, 3 Jul 2025 18:19:29 +0530
Subject: [PATCH] fix: improved members list and form

---
 .../Modals/CourseProgressSummary.vue          |   2 +-
 frontend/src/components/Settings/Members.vue  | 128 ++++++++++--------
 frontend/src/components/Settings/Settings.vue |   3 +-
 lms/lms/api.py                                |  11 +-
 4 files changed, 76 insertions(+), 68 deletions(-)

diff --git a/frontend/src/components/Modals/CourseProgressSummary.vue b/frontend/src/components/Modals/CourseProgressSummary.vue
index 6ea915b7..c87de333 100644
--- a/frontend/src/components/Modals/CourseProgressSummary.vue
+++ b/frontend/src/components/Modals/CourseProgressSummary.vue
@@ -206,7 +206,7 @@ const progressColumns = computed(() => {
 		{
 			label: __('Member'),
 			key: 'member_name',
-			width: '50%',
+			width: '60%',
 			icon: 'user',
 		},
 		{
diff --git a/frontend/src/components/Settings/Members.vue b/frontend/src/components/Settings/Members.vue
index 730758d7..74bc4b5d 100644
--- a/frontend/src/components/Settings/Members.vue
+++ b/frontend/src/components/Settings/Members.vue
@@ -5,9 +5,9 @@
 				<div class="text-xl font-semibold mb-1 text-ink-gray-9">
 					{{ __(label) }}
 				</div>
-				<!-- <div class="text-xs text-ink-gray-5">
+				<div class="text-xs text-ink-gray-5 leading-5">
 					{{ __(description) }}
-				</div> -->
+				</div>
 			</div>
 			<div class="flex item-center space-x-2">
 				<FormControl
@@ -18,40 +18,20 @@
 				/>
 				<Button @click="() => (showForm = !showForm)">
 					<template #prefix>
-						<Plus v-if="!showForm" class="size-4 stroke-1.5" />
-						<X v-else class="size-4 stroke-1.5" />
+						<Plus class="size-4 stroke-1.5" />
 					</template>
-					{{ showForm ? __('Close') : __('New') }}
+					{{ __('New') }}
 				</Button>
 			</div>
 		</div>
 
-		<!-- Form to add new member -->
-		<div v-if="showForm" class="flex items-center space-x-2 my-4">
-			<FormControl
-				v-model="member.email"
-				:placeholder="__('Email')"
-				type="email"
-				class="w-full"
-			/>
-			<FormControl
-				v-model="member.first_name"
-				:placeholder="__('First Name')"
-				type="text"
-				class="w-full"
-			/>
-			<Button @click="addMember()" variant="subtle">
-				{{ __('Add') }}
-			</Button>
-		</div>
-
 		<div class="mt-2 pb-10 overflow-auto">
 			<!-- Member list -->
 			<div class="overflow-y-scroll">
 				<ul class="divide-y">
 					<li
 						v-for="member in memberList"
-						class="grid grid-cols-3 gap-10 py-2 cursor-pointer"
+						class="flex items-center justify-between py-2 cursor-pointer"
 					>
 						<div
 							@click="openProfile(member.username)"
@@ -60,27 +40,13 @@
 							<Avatar
 								:image="member.user_image"
 								:label="member.full_name"
-								size="lg"
+								size="xl"
 							/>
 							<div class="space-y-1">
 								<div class="flex">
 									<div class="text-ink-gray-9">
 										{{ member.full_name }}
 									</div>
-									<div
-										class="px-1"
-										v-if="member.role && getRole(member.role) !== 'Student'"
-									>
-										<Badge
-											:variant="'subtle'"
-											:ref_for="true"
-											theme="blue"
-											size="sm"
-											label="Badge"
-										>
-											{{ getRole(member.role) }}
-										</Badge>
-									</div>
 								</div>
 								<div class="text-sm text-ink-gray-7">
 									{{ member.name }}
@@ -88,12 +54,13 @@
 							</div>
 						</div>
 						<div
-							class="flex items-center justify-center text-ink-gray-7 text-sm"
+							class="flex items-center space-x-1 bg-surface-gray-2 px-2 py-1.5 rounded-md"
+							v-if="member.role && member.role !== 'LMS Student'"
 						>
-							<div v-if="member.last_active">
-								{{ dayjs(member.last_active).format('DD MMM, YYYY HH:mm a') }}
-							</div>
-							<div v-else>-</div>
+							<Shield class="size-4 stroke-1.5" />
+							<span>
+								{{ getRole(member.role) }}
+							</span>
 						</div>
 					</li>
 				</ul>
@@ -111,20 +78,68 @@
 			</div>
 		</div>
 	</div>
+	<Dialog
+		v-model="showForm"
+		:options="{
+			title: __('Add a new member'),
+			size: 'lg',
+			actions: [{
+				label: __('Add'),
+				variant: 'solid',
+				onClick({ close }: any) {
+					addMember(close)
+				}
+			}]
+		}"
+	>
+		<template #body-content>
+			<div class="flex items-center space-x-2">
+				<FormControl
+					v-model="member.email"
+					:label="__('Email')"
+					placeholder="jane@doe.com"
+					type="email"
+					class="w-full"
+				/>
+				<FormControl
+					v-model="member.first_name"
+					:label="__('First Name')"
+					placeholder="Jane"
+					type="text"
+					class="w-full"
+				/>
+			</div>
+		</template>
+	</Dialog>
 </template>
 <script setup lang="ts">
-import { createResource, Avatar, Button, FormControl, Badge } from 'frappe-ui'
+import {
+	Avatar,
+	Badge,
+	Button,
+	createResource,
+	Dialog,
+	FormControl,
+} from 'frappe-ui'
 import { useRouter } from 'vue-router'
 import { ref, watch, reactive, inject } from 'vue'
-import { RefreshCw, Plus, X } from 'lucide-vue-next'
+import { RefreshCw, Plus, X, Shield } from 'lucide-vue-next'
 import { useOnboarding } from 'frappe-ui/frappe'
 import type { User } from '@/components/Settings/types'
 
+type Member = {
+	username: string
+	full_name: string
+	name: string
+	role?: string
+	user_image?: string
+}
+
 const router = useRouter()
 const show = defineModel('show')
 const search = ref('')
 const start = ref(0)
-const memberList = ref([])
+const memberList = ref<Member[]>([])
 const hasNextPage = ref(false)
 const showForm = ref(false)
 const dayjs = inject('$dayjs')
@@ -158,7 +173,7 @@ const members = createResource({
 			start: start.value,
 		}
 	},
-	onSuccess(data) {
+	onSuccess(data: Member[]) {
 		memberList.value = memberList.value.concat(data)
 		start.value = start.value + 20
 		hasNextPage.value = data.length === 20
@@ -166,7 +181,7 @@ const members = createResource({
 	auto: true,
 })
 
-const openProfile = (username) => {
+const openProfile = (username: string) => {
 	show.value = false
 	router.push({
 		name: 'Profile',
@@ -178,7 +193,7 @@ const openProfile = (username) => {
 
 const newMember = createResource({
 	url: 'frappe.client.insert',
-	makeParams(values) {
+	makeParams() {
 		return {
 			doc: {
 				doctype: 'User',
@@ -188,13 +203,13 @@ const newMember = createResource({
 		}
 	},
 	auto: false,
-	onSuccess(data) {
+	onSuccess(data: Member) {
 		show.value = false
 
 		if (user?.data?.is_system_manager) updateOnboardingStep('invite_students')
 
 		router.push({
-			name: 'Profile',
+			name: 'ProfileRoles',
 			params: {
 				username: data.username,
 			},
@@ -202,8 +217,9 @@ const newMember = createResource({
 	},
 })
 
-const addMember = () => {
+const addMember = (close: () => void) => {
 	newMember.reload()
+	close()
 }
 
 watch(search, () => {
@@ -212,8 +228,8 @@ watch(search, () => {
 	members.reload()
 })
 
-const getRole = (role) => {
-	const map = {
+const getRole = (role: string) => {
+	const map: Record<string, string> = {
 		'LMS Student': 'Student',
 		'Course Creator': 'Instructor',
 		Moderator: 'Moderator',
diff --git a/frontend/src/components/Settings/Settings.vue b/frontend/src/components/Settings/Settings.vue
index 580a649c..693e2640 100644
--- a/frontend/src/components/Settings/Settings.vue
+++ b/frontend/src/components/Settings/Settings.vue
@@ -242,7 +242,8 @@ const tabsStructure = computed(() => {
 			items: [
 				{
 					label: 'Members',
-					description: 'Manage the members of your learning system',
+					description:
+						'Add new members or manage roles and permissions of existing members',
 					icon: 'UserRoundPlus',
 				},
 				{
diff --git a/lms/lms/api.py b/lms/lms/api.py
index d12dc507..35f50b0f 100644
--- a/lms/lms/api.py
+++ b/lms/lms/api.py
@@ -696,15 +696,6 @@ def get_categories(doctype, filters):
 
 @frappe.whitelist()
 def get_members(start=0, search=""):
-	"""Get members for the given search term and start index.
-	                                                                Args: start (int): Start index for the query.
-	<<<<<<< HEAD
-	                                                                search (str): Search term to filter the results.
-	=======
-	                                                                                                                                                                                                                                                                                                                                search (str): Search term to filter the results.
-	>>>>>>> 4869bba7bbb2fb38477d6fc29fb3b5838e075577
-	                                                                Returns: List of members.
-	"""
 
 	filters = {"enabled": 1, "name": ["not in", ["Administrator", "Guest"]]}
 	or_filters = {}
@@ -1574,7 +1565,7 @@ def track_video_watch_duration(lesson, videos):
 		existing_record = frappe.db.get_value(
 			"LMS Video Watch Duration", filters, ["name", "watch_time"], as_dict=True
 		)
-		if existing_record and existing_record.watch_time < video.get("watch_time"):
+		if existing_record and flt(existing_record.watch_time) < flt(video.get("watch_time")):
 			frappe.db.set_value(
 				"LMS Video Watch Duration",
 				filters,