From 84f40fc53fde6b24949b9065f494fa4926d7631e Mon Sep 17 00:00:00 2001 From: Markus Stetschnig Date: Tue, 16 Apr 2024 11:53:07 +0200 Subject: [PATCH 1/4] SAK-49998 Assignments: Improve performance and UX of Assignments by student view --- .../assignment/api/AssignmentConstants.java | 5 + .../api/src/resources/assignment.properties | 18 +++ .../src/resources/assignment_ca.properties | 18 +++ .../src/resources/assignment_es.properties | 18 +++ .../assignment/tool/AssignmentAction.java | 102 ++++++++------ .../src/webapp/js/assignmentsByStudent.js | 75 ++++++++++ ...nts_instructor_student_list_submissions.vm | 132 +++++++++++------- .../tool/assignments/_assignments.scss | 6 +- library/src/skins/default/src/sass/tool.scss | 1 + .../tool/src/templates/VM_chef_library.vm | 47 +++++++ 10 files changed, 332 insertions(+), 90 deletions(-) create mode 100644 assignment/tool/src/webapp/js/assignmentsByStudent.js diff --git a/assignment/api/src/java/org/sakaiproject/assignment/api/AssignmentConstants.java b/assignment/api/src/java/org/sakaiproject/assignment/api/AssignmentConstants.java index b18bd221cbf6..632f7b256188 100644 --- a/assignment/api/src/java/org/sakaiproject/assignment/api/AssignmentConstants.java +++ b/assignment/api/src/java/org/sakaiproject/assignment/api/AssignmentConstants.java @@ -21,6 +21,8 @@ package org.sakaiproject.assignment.api; +import java.util.List; + /** * Store the constants used by Assignment tool and service * @@ -385,5 +387,8 @@ public enum IMSGradingProgress { public static final String SAK_PROP_ALLOW_LINK_TO_EXISTING_GB_ITEM = "assignment.allowLinkToExistingGBItem"; public static final boolean SAK_PROP_ALLOW_LINK_TO_EXISTING_GB_ITEM_DFLT = true; + public static final String SAK_PROP_NON_SUBMITTER_PERMISSIONS = "assignment.submitter.remove.permission"; + public static final List SAK_PROP_NON_SUBMITTER_PERMISSIONS_DEFAULT = List.of(AssignmentServiceConstants.SECURE_ADD_ASSIGNMENT); + public static final String ASSIGNMENT_INPUT_ADD_SUBMISSION_TIME_SPENT = "value_ASSIGNMENT_INPUT_ADD_SUBMISSION_TIME_SPENT"; } diff --git a/assignment/api/src/resources/assignment.properties b/assignment/api/src/resources/assignment.properties index 8764d52af7ae..90b9dac10675 100644 --- a/assignment/api/src/resources/assignment.properties +++ b/assignment/api/src/resources/assignment.properties @@ -1311,3 +1311,21 @@ tags_option_students_default=Default (hide tags to students) tags_option_students_yes=Show tags to students and allow them to use the filter component select=Select + +datatables.aria.orderable=Activate to sort +datatables.aria.orderableRemove=Activate to remove sorting +datatables.aria.orderableReverse=Activate to invert sorting +datatables.aria.sortAscending=Activate to sort column ascending +datatables.aria.sortDescending=Activate to sort column descending +datatables.custom.first=First +datatables.custom.last=Last +datatables.custom.next=Next +datatables.custom.previous=Previous +datatables.emptyTable=No entries found +datatables.entries=entries +datatables.info=Showing _START_ to _END_ of _TOTAL_ entries +datatables.infoEmpty=Showing 0 to 0 of 0 entries +datatables.infoFiltered=(filtered from _MAX_ total entries) +datatables.lengthMenu=Show _MENU_ entries +datatables.search=Search: +datatables.zeroRecords=No matching entries found diff --git a/assignment/api/src/resources/assignment_ca.properties b/assignment/api/src/resources/assignment_ca.properties index 3efbd47db1e2..4ac6d5f42ec8 100644 --- a/assignment/api/src/resources/assignment_ca.properties +++ b/assignment/api/src/resources/assignment_ca.properties @@ -1281,3 +1281,21 @@ tags_search=Cercar usant etiquetes tags_option_students=Mostrar etiquetes als estudiants tags_option_students_default=Predeterminat (ocultar etiquetes als estudiants). tags_option_students_yes=Mostrar etiquetes als estudiantes i permetre'ls usar el filtre. + +datatables.aria.orderable=Activar per ordenar +datatables.aria.orderableRemove=Activar per eliminar l\u0027ordenaci\u00f3 +datatables.aria.orderableReverse=Activar per invertir l\u0027ordenaci\u00f3 +datatables.aria.sortAscending=Activar per ordenar columna de forma ascendent +datatables.aria.sortDescending=Activar per ordenar columna de forma descendent +datatables.custom.first=Primer +datatables.custom.last=\u00daltim +datatables.custom.next=Seg\u00fcent +datatables.custom.previous=Anterior +datatables.emptyTable=No s\u0027han trobat entrades +datatables.entries=entrades +datatables.info=Mostrant _START_ a _END_ de _TOTAL_ entrades +datatables.infoEmpty=Mostrant 0 a 0 de 0 entrades +datatables.infoFiltered=(filtrat de _MAX_ total entrades) +datatables.lengthMenu=Mostrar _MENU_ entrades +datatables.search=Cercar: +datatables.zeroRecords=No s\u0027han trobat entrades coincidents diff --git a/assignment/api/src/resources/assignment_es.properties b/assignment/api/src/resources/assignment_es.properties index 650cb1f0bae2..f9a334d04cfe 100644 --- a/assignment/api/src/resources/assignment_es.properties +++ b/assignment/api/src/resources/assignment_es.properties @@ -1274,3 +1274,21 @@ tags_search=Buscar usando etiquetas tags_option_students=Mostrar etiquetas a los estudiantes. tags_option_students_default=Por defecto (ocultar etiquetas a los estudiantes). tags_option_students_yes=Mostrar etiquetas a los estudiantes y permitirles el uso del filtro. + +datatables.aria.orderable=Activar para ordenar +datatables.aria.orderableRemove=Activar para quitar ordenaci\u00f3n +datatables.aria.orderableReverse=Activar para invertir ordenaci\u00f3n +datatables.aria.sortAscending=Activar para ordenar columna de forma ascendente +datatables.aria.sortDescending=Activar para ordenar columna de forma descendente +datatables.custom.first=Primero +datatables.custom.last=\u00daltimo +datatables.custom.next=Siguiente +datatables.custom.previous=Anterior +datatables.emptyTable=No se encontraron entradas +datatables.entries=entradas +datatables.info=Mostrando _START_ a _END_ de _TOTAL_ entradas +datatables.infoEmpty=Mostrando 0 a 0 de 0 entradas +datatables.infoFiltered=(filtrado de _MAX_ total entradas) +datatables.lengthMenu=Mostrar _MENU_ entradas +datatables.search=Buscar: +datatables.zeroRecords=No se encontraron entradas coincidentes diff --git a/assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java b/assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java index 46ce0fb489c5..0ef37b940d25 100644 --- a/assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java +++ b/assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java @@ -108,6 +108,7 @@ import java.util.SortedSet; import java.util.concurrent.ConcurrentSkipListSet; import java.util.function.Function; +import java.util.function.Predicate; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -1086,6 +1087,10 @@ public class AssignmentAction extends PagedResourceActionII { * To know if grade_submission go from view_students_assignment view or not **/ private static final String FROM_VIEW = "from_view"; + /** + * Whether or not to reset the table state stored on the frontend + */ + private static final String RESET_TABLE_STATE = "reset_table_state"; /** * Site property for export rubric in pdf @@ -5712,18 +5717,27 @@ private String build_instructor_view_students_assignment_context(VelocityPortlet // cleaning from view attribute state.removeAttribute(FROM_VIEW); + // Null or true means we want to reset the tables state + boolean resetTableState = !Boolean.FALSE.equals(state.getAttribute(RESET_TABLE_STATE)); + context.put("resetTableState", resetTableState); + state.setAttribute(RESET_TABLE_STATE, false); + String contextString = (String) state.getAttribute(STATE_CONTEXT_STRING); + Site site; + try { + site = siteService.getSite(contextString); + } catch (IdUnusedException e) { + throw new IllegalStateException("Can not build context for invalid site with id [" + contextString + "]"); + } initViewSubmissionListOption(state); String allOrOneGroup = (String) state.getAttribute(VIEW_SUBMISSION_LIST_OPTION); - String search = (String) state.getAttribute(VIEW_SUBMISSION_SEARCH); Boolean searchFilterOnly = (state.getAttribute(SUBMISSIONS_SEARCH_ONLY) != null && ((Boolean) state.getAttribute(SUBMISSIONS_SEARCH_ONLY)) ? Boolean.TRUE : Boolean.FALSE); String accessPointUrl = serverConfigurationService.getAccessUrl() + AssignmentReferenceReckoner.reckoner().context(contextString).reckon().getReference() + "?contextString=" + contextString + "&viewString=" + allOrOneGroup + - "&searchString=" + search + "&searchFilterOnly=" + searchFilterOnly.toString() + "&estimate=true"; context.put("accessPointUrl", accessPointUrl); @@ -5739,53 +5753,55 @@ private String build_instructor_view_students_assignment_context(VelocityPortlet } context.put("hasAtLeastOneAnonAssignment", hasAtLeastOneAnonAssigment); - Map studentMembers = assignments.stream() - // flatten to a single List - .flatMap(a -> assignmentService.getSubmitterIdList(searchFilterOnly.toString(), allOrOneGroup, search, AssignmentReferenceReckoner.reckoner().assignment(a).reckon().getReference(), contextString).stream()) - // collect into set for uniqueness - .collect(Collectors.toSet()).stream() - // convert to User - .map(s -> { - try { - return userDirectoryService.getUser(s); - } catch (UserNotDefinedException e) { - log.warn("User is not defined {}, {}", s, e.getMessage()); - return null; - } - }) - // filter nulls - .filter(Objects::nonNull) - // collect to Map - .collect(Collectors.toMap(User::getId, Function.identity())); + List nonSubmitterPermissions = serverConfigurationService.getStringList(AssignmentConstants.SAK_PROP_NON_SUBMITTER_PERMISSIONS, + AssignmentConstants.SAK_PROP_NON_SUBMITTER_PERMISSIONS_DEFAULT); - context.put("studentMembersMap", studentMembers); - context.put("studentMembers", new SortedIterator(studentMembers.values().iterator(), new AssignmentComparator(state, SORTED_USER_BY_SORTNAME, Boolean.TRUE.toString()))); - context.put("viewGroup", state.getAttribute(VIEW_SUBMISSION_LIST_OPTION)); - context.put("searchString", state.getAttribute(VIEW_SUBMISSION_SEARCH) != null ? state.getAttribute(VIEW_SUBMISSION_SEARCH) : ""); - context.put("showSubmissionByFilterSearchOnly", state.getAttribute(SUBMISSIONS_SEARCH_ONLY) != null ? (Boolean) state.getAttribute(SUBMISSIONS_SEARCH_ONLY) : Boolean.FALSE); - Collection groups = getAllGroupsInSite(contextString); - context.put("groups", new SortedIterator(groups.iterator(), new AssignmentComparator(state, SORTED_BY_GROUP_TITLE, Boolean.TRUE.toString()))); + Predicate isNonSubmitter = (userId) -> nonSubmitterPermissions.stream() + .filter(permission -> securityService.unlock(userId, permission, site.getReference())) + .findAny() + .isEmpty(); + + AuthzGroup groupSelection = StringUtils.isBlank(allOrOneGroup) || AssignmentConstants.ALL.equals(allOrOneGroup) + ? site + : site.getGroup(allOrOneGroup); + + Map studentMembers = groupSelection.getUsers().stream() + .filter(isNonSubmitter) + .map(this::getUser) + .flatMap(Optional::stream) + .collect(Collectors.toMap(User::getId, Function.identity())); + + Comparator assignmentComparator = new AssignmentComparator(state, SORTED_BY_DEFAULT, Boolean.TRUE.toString()); Map> showStudentAssignments = new HashMap<>(); - Set showStudentListSet = (Set) state.getAttribute(STUDENT_LIST_SHOW_TABLE); - if (showStudentListSet != null) { - context.put("studentListShowSet", showStudentListSet); - for (String userId : showStudentListSet) { - User user = studentMembers.get(userId); + Set expandedStudents = (Set) state.getAttribute(STUDENT_LIST_SHOW_TABLE); + if (expandedStudents != null) { + context.put("studentListShowSet", expandedStudents); - // filter to obtain only grade-able assignments - List rv = assignments.stream() - .filter(a -> assignmentService.allowGradeSubmission(AssignmentReferenceReckoner.reckoner().assignment(a).reckon().getReference())) - .collect(Collectors.toList()); + List gradableAssignments = assignments.stream() + .filter(a -> assignmentService.allowGradeSubmission(AssignmentReferenceReckoner.reckoner().assignment(a).reckon().getReference())) + .sorted(assignmentComparator) + .collect(Collectors.toList()); - // sort the assignments into the default order before adding - Iterator assignmentSortFinal = new SortedIterator(rv.iterator(), new AssignmentComparator(state, SORTED_BY_DEFAULT, Boolean.TRUE.toString())); + for (String userId : expandedStudents) { + List userSubmittableAssignments = gradableAssignments.stream() + .filter(assignment -> assignmentService.canSubmit(assignment, userId)) + .collect(Collectors.toList()); - showStudentAssignments.put(user, assignmentSortFinal); + showStudentAssignments.put(studentMembers.get(userId), userSubmittableAssignments.iterator()); } } + context.put("studentMembersMap", studentMembers); + context.put("studentMembers", new SortedIterator(studentMembers.values().iterator(), new AssignmentComparator(state, SORTED_USER_BY_SORTNAME, Boolean.TRUE.toString()))); + context.put("viewGroup", state.getAttribute(VIEW_SUBMISSION_LIST_OPTION)); + context.put("searchString", state.getAttribute(VIEW_SUBMISSION_SEARCH) != null ? state.getAttribute(VIEW_SUBMISSION_SEARCH) : ""); + context.put("showSubmissionByFilterSearchOnly", state.getAttribute(SUBMISSIONS_SEARCH_ONLY) != null ? (Boolean) state.getAttribute(SUBMISSIONS_SEARCH_ONLY) : Boolean.FALSE); + Collection groups = getAllGroupsInSite(contextString); + context.put("groups", new SortedIterator(groups.iterator(), new AssignmentComparator(state, SORTED_BY_GROUP_TITLE, Boolean.TRUE.toString()))); + + context.put("studentAssignmentsTable", showStudentAssignments); context.put("currentTime", Instant.now()); @@ -16766,4 +16782,12 @@ private String getRateSubmissionTimeSpent(AssignmentSubmission submission) { } return assignmentService.intToTime(assigmentRateSpent); } + + private Optional getUser(String userId) { + try { + return Optional.of(userDirectoryService.getUser(userId)); + } catch (UserNotDefinedException e) { + return Optional.empty(); + } + } } diff --git a/assignment/tool/src/webapp/js/assignmentsByStudent.js b/assignment/tool/src/webapp/js/assignmentsByStudent.js new file mode 100644 index 000000000000..2b2b9eb3165a --- /dev/null +++ b/assignment/tool/src/webapp/js/assignmentsByStudent.js @@ -0,0 +1,75 @@ +(() => { + +// Include datatables dependencies +window.includeWebjarLibrary('datatables'); +window.includeWebjarLibrary('datatables-rowgroup'); + +// Make sure assignments namespace exists +window.assignments = window.assignments ?? {}; + +// Assignments By Students "global" namespace +window.assignments.byStudent = { + datatablesConfig: { + dom: '<<".dt-header-row"<".dt-header-slot">lf><".dt-footer-row"ip>>', + stateSave: true, + columnDefs: [ + { + sortable: false, + targets: "no-sort", + }, + ], + rowGroup: { + dataSrc(row) { + const dataCellHtml = row[0].display; + return parseDataCell(dataCellHtml).studentUserId; + }, + startRender(rows, group) { + const firstRow = rows.data()[0]; + const dataCellHtml = firstRow[0].display; + + const data = parseDataCell(dataCellHtml); + + return renderGrouping(data); + }, + }, + }, +} + +// Private functions + +function parseDataCell(html) { + const template = document.createElement('template'); + template.innerHTML = html; + const cell = template.content.children[0]; + + const expanded = cell.getAttribute("data-expanded") == "true"; + const actionLink = cell.getAttribute("data-action-href"); + const studentUserId = cell.getAttribute("data-user-id"); + const studentName = cell.innerText.trim(); + + return { + actionLink, + expanded, + studentName, + studentUserId, + }; +} + +function renderGrouping({ studentName, actionLink, expanded }) { + const template = document.createElement('template'); + template.innerHTML = ` + + + + + ${studentName} + + + + `; + + return template.content; +} + +})(); diff --git a/assignment/tool/src/webapp/vm/assignment/chef_assignments_instructor_student_list_submissions.vm b/assignment/tool/src/webapp/vm/assignment/chef_assignments_instructor_student_list_submissions.vm index 240c89470d04..c310189cceb2 100644 --- a/assignment/tool/src/webapp/vm/assignment/chef_assignments_instructor_student_list_submissions.vm +++ b/assignment/tool/src/webapp/vm/assignment/chef_assignments_instructor_student_list_submissions.vm @@ -1,4 +1,35 @@ +#javascript("/sakai-assignment-tool/js/assignmentsByStudent.js") +
#navBarHREF( $allowAddAssignment $allowGradeSubmission $allowAddAssignment $allowRecoverAssignment $allowAllGroups $assignmentscheck $allowUpdateSite $enableViewOption $view "" )
@@ -28,9 +59,6 @@ #end #viewFilterPanel("viewFilterForm", "viewgroup", $viewSelectOptions, "doChange_submission_list_option") #end - #if( !$hasAtLeastOneAnonAssignment ) - #searchFilterPanel("$form_search", $searchString, "doView_submission_list_search", "doView_submission_list_search_clear") - #end
#if ($!isTimesheet) @@ -46,7 +74,8 @@ $tlang.getString("gen.theare2")

#else - +
+ - #if ($!isTimesheet) - #end - + + #foreach ($member in $studentMembers) #set($submitterName=$!member.sortName) #set($submitterId=$!member.getDisplayId()) + #set($searchString = $studentSearchMap.get($member.Id)) + #set($isExpanded = $studentListShowSet.contains($member.Id)) #if ($!submitterId) ##attach the displayId #set($submitterName=$submitterName.concat(" (").concat($submitterId).concat(")")) - #end - - + - - - #if ($studentListShowSet.contains($member.Id)) + data-expanded="$isExpanded" + > + $submitterName + + + + + + #if ($!isTimesheet) + + #end + + + #else #set($assignments=false) #set($assignments=$!studentAssignmentsTable.get($member)) #foreach ($assignment in $!assignments) #set ($assignmentReference = $!service.assignmentReference($assignment.Id)) #set ($isAnon = $!service.assignmentUsesAnonymousGrading($assignment)) + #set($assignmentTitle = $formattedText.escapeHtml($assignment.Title)) #if (!$assignment.Draft) ## do not show draft assignments #set($submission = false) #set($submission=$service.getSubmission($assignment.Id, $member)) #set($submissionReference=$service.submissionReference($assignment.Context, $submission.Id, $assignment.Id)) - + - #if ($!isTimesheet) @@ -222,6 +253,7 @@ #end #end #end +
$tlang.getString("gen.student") @@ -54,80 +83,83 @@ $tlang.getString("gen.assig") + $tlang.getString("gen.subm4") $tlang.getString("gen.status") + $tlang.getString("gen.spenttime") + $tlang.getString("gen.gra")
-
-
- #if ($isAnon || $assignment.IsGroup) - + #end + #set($submitterName=$formattedText.escapeHtml($submitterName)) + #if (!$isExpanded || !$!studentAssignmentsTable.get($member).hasNext()) +
- + + + $submitterName + + #if (!$isAnon) - $formattedText.escapeHtml($assignment.Title) + $assignmentTitle #if ($allowAddAssignment && $allowSubmitByInstructor) #set( $submitSpinnerID = "submitFor_" + $member.Id + "_" + $formattedText.escapeUrl($assignmentReference) )
@@ -142,7 +174,7 @@
#end #else - $formattedText.escapeHtml($assignment.Title) ($tlang.getString("grading.anonymous.title")) + $assignmentTitle ($tlang.getString("grading.anonymous.title")) #end 		@@ -168,12 +200,11 @@ #end   - #set($submissionId=false) + #if ($submission) #set($submissionId = $submission.Id) + $!service.getSubmissionStatus($!submissionId, true) #end - $!service.getSubmissionStatus($!submissionId, true)  
#end diff --git a/library/src/skins/default/src/sass/modules/tool/assignments/_assignments.scss b/library/src/skins/default/src/sass/modules/tool/assignments/_assignments.scss index ed27b851dc19..909215c57e32 100644 --- a/library/src/skins/default/src/sass/modules/tool/assignments/_assignments.scss +++ b/library/src/skins/default/src/sass/modules/tool/assignments/_assignments.scss @@ -688,5 +688,9 @@ div:first-child { margin-right: 7px; } - } + + .expand-icon { + width: 1em; + text-align: center; + } } diff --git a/library/src/skins/default/src/sass/tool.scss b/library/src/skins/default/src/sass/tool.scss index 4a1f69ac7d88..d5785fb889f2 100644 --- a/library/src/skins/default/src/sass/tool.scss +++ b/library/src/skins/default/src/sass/tool.scss @@ -199,6 +199,7 @@ $jumbotron-heading-font-size: $h4-font-size; @import "modules/tool/sakai-options-menu/sakai-options-menu"; @import "modules/tool/access"; @import "modules/tool/search/search"; +@import "modules/datatables/base"; diff --git a/velocity/tool/src/templates/VM_chef_library.vm b/velocity/tool/src/templates/VM_chef_library.vm index 6e23953d7f39..c3ce5c943e5c 100644 --- a/velocity/tool/src/templates/VM_chef_library.vm +++ b/velocity/tool/src/templates/VM_chef_library.vm @@ -705,3 +705,50 @@ ${extra}") #set($libLink="#libraryLink('js/sakai-table-toolbar/searchFilterPanelMacro.js')") #end ## searchFilterPanel + +#* ----------------------------------------------------------------------------------- +# Adds an object with a common datatables config +# ----------------------------------------------------------------------------------- +*# +#macro (datatablesCommonConfig) +{ + pageLength: 20, + lengthMenu: [ 5, 10, 20, 50, 100, 200 ], +} +#end + +#* ----------------------------------------------------------------------------------- +# Adds an language object for the datatables library +# ----------------------------------------------------------------------------------- +*# +#macro (datatablesBundle $resourceLoader) +{ + emptyTable: "$resourceLoader.getString('datatables.emptyTable')", + entries: "$resourceLoader.getString('datatables.entries')", + info: "$resourceLoader.getString('datatables.info')", + infoEmpty: "$resourceLoader.getString('datatables.infoEmpty')", + infoFiltered: "$resourceLoader.getString('datatables.infoFiltered')", + lengthMenu: "$resourceLoader.getString('datatables.lengthMenu')", + search: "$resourceLoader.getString('datatables.search')", + zeroRecords: "$resourceLoader.getString('datatables.zeroRecords')", + aria: { + orderable: "$resourceLoader.getString('datatables.aria.orderable')", + orderableRemove: "$resourceLoader.getString('datatables.aria.orderableRemove')", + orderableReverse: "$resourceLoader.getString('datatables.aria.orderableReverse')", + sortAscending: "$resourceLoader.getString('datatables.aria.sortAscending')", + sortDescending: "$resourceLoader.getString('datatables.aria.sortDescending')", + placeholder: { + first: "$resourceLoader.getString('datatables.custom.first')", + last: "$resourceLoader.getString('datatables.custom.last')", + next: "$resourceLoader.getString('datatables.custom.next')", + previous: "$resourceLoader.getString('datatables.custom.previous')", + }, + }, + placeholder: { + first: "$resourceLoader.getString('datatables.custom.first')", + last: "$resourceLoader.getString('datatables.custom.last')", + next: "$resourceLoader.getString('datatables.custom.next')", + previous: "$resourceLoader.getString('datatables.custom.previous')", + }, +} +#end From a03ff59a05d636930b5290de4a6edb8ed7a7c6f3 Mon Sep 17 00:00:00 2001 From: Markus Stetschnig Date: Wed, 24 Apr 2024 14:55:36 +0200 Subject: [PATCH 2/4] SAK-49998 Master adjustments --- .../sakaiproject/assignment/tool/AssignmentAction.java | 10 +--------- assignment/tool/src/webapp/js/assignmentsByStudent.js | 4 ++-- .../sakaiproject/user/api/UserDirectoryService.java | 9 +++++++++ .../user/impl/BaseUserDirectoryService.java | 9 +++++++++ library/src/skins/default/src/sass/base/_icons.scss | 2 ++ .../default/src/sass/modules/datatables/_base.scss | 10 ++++++++++ .../sass/modules/tool/assignments/_assignments.scss | 9 +++++---- 7 files changed, 38 insertions(+), 15 deletions(-) create mode 100644 library/src/skins/default/src/sass/modules/datatables/_base.scss diff --git a/assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java b/assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java index 0ef37b940d25..b3f491fa093a 100644 --- a/assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java +++ b/assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java @@ -5767,7 +5767,7 @@ private String build_instructor_view_students_assignment_context(VelocityPortlet Map studentMembers = groupSelection.getUsers().stream() .filter(isNonSubmitter) - .map(this::getUser) + .map(userDirectoryService::getOptionalUser) .flatMap(Optional::stream) .collect(Collectors.toMap(User::getId, Function.identity())); @@ -16782,12 +16782,4 @@ private String getRateSubmissionTimeSpent(AssignmentSubmission submission) { } return assignmentService.intToTime(assigmentRateSpent); } - - private Optional getUser(String userId) { - try { - return Optional.of(userDirectoryService.getUser(userId)); - } catch (UserNotDefinedException e) { - return Optional.empty(); - } - } } diff --git a/assignment/tool/src/webapp/js/assignmentsByStudent.js b/assignment/tool/src/webapp/js/assignmentsByStudent.js index 2b2b9eb3165a..da53f6e16003 100644 --- a/assignment/tool/src/webapp/js/assignmentsByStudent.js +++ b/assignment/tool/src/webapp/js/assignmentsByStudent.js @@ -59,9 +59,9 @@ function renderGrouping({ studentName, actionLink, expanded }) { const template = document.createElement('template'); template.innerHTML = ` - + - ${studentName} diff --git a/kernel/api/src/main/java/org/sakaiproject/user/api/UserDirectoryService.java b/kernel/api/src/main/java/org/sakaiproject/user/api/UserDirectoryService.java index 9cdfa1f55f49..25b345590bea 100644 --- a/kernel/api/src/main/java/org/sakaiproject/user/api/UserDirectoryService.java +++ b/kernel/api/src/main/java/org/sakaiproject/user/api/UserDirectoryService.java @@ -27,6 +27,7 @@ import java.util.Collection; import java.util.List; +import java.util.Optional; /** *

@@ -351,6 +352,14 @@ User addUser(String id, String eid, String firstName, String lastName, String em */ User getUser(String id) throws UserNotDefinedException; + /** + * Access a user object as an Optional. + * + * @param userId The user id string. + * @return A user object containing the user information wrapped in an Optional + */ + Optional getOptionalUser(String userId); + /** * Access a user object, given an enterprise id. * diff --git a/kernel/kernel-impl/src/main/java/org/sakaiproject/user/impl/BaseUserDirectoryService.java b/kernel/kernel-impl/src/main/java/org/sakaiproject/user/impl/BaseUserDirectoryService.java index ea4a1ea79a8b..4876732bca02 100644 --- a/kernel/kernel-impl/src/main/java/org/sakaiproject/user/impl/BaseUserDirectoryService.java +++ b/kernel/kernel-impl/src/main/java/org/sakaiproject/user/impl/BaseUserDirectoryService.java @@ -31,6 +31,7 @@ import java.util.Map; import java.util.Observable; import java.util.Observer; +import java.util.Optional; import java.util.Set; import java.util.Stack; import java.util.TreeSet; @@ -882,6 +883,14 @@ public User getUser(String id) throws UserNotDefinedException return user; } + public Optional getOptionalUser(String userId) { + try { + return Optional.of(getUser(userId)); + } catch (UserNotDefinedException e) { + return Optional.empty(); + } + } + /** * @inheritDoc */ diff --git a/library/src/skins/default/src/sass/base/_icons.scss b/library/src/skins/default/src/sass/base/_icons.scss index c8986e955f67..d3c0f890989d 100644 --- a/library/src/skins/default/src/sass/base/_icons.scss +++ b/library/src/skins/default/src/sass/base/_icons.scss @@ -184,6 +184,8 @@ $fa-font-path: "./fonts"; eye-slash-fill: bi-eye-slash-fill, question: bi-question, warning: bi-exclamation-lg, + expanded: bi-chevron-down, + collapsed: bi-chevron-right, ); diff --git a/library/src/skins/default/src/sass/modules/datatables/_base.scss b/library/src/skins/default/src/sass/modules/datatables/_base.scss new file mode 100644 index 000000000000..aafc188adf71 --- /dev/null +++ b/library/src/skins/default/src/sass/modules/datatables/_base.scss @@ -0,0 +1,10 @@ +.dt-header-row, +.dt-footer-row { + display: flex; + justify-content: space-between; + flex-wrap: wrap; +} + +.dt-header-slot { + padding: 2px; +} diff --git a/library/src/skins/default/src/sass/modules/tool/assignments/_assignments.scss b/library/src/skins/default/src/sass/modules/tool/assignments/_assignments.scss index 909215c57e32..ba0397211a53 100644 --- a/library/src/skins/default/src/sass/modules/tool/assignments/_assignments.scss +++ b/library/src/skins/default/src/sass/modules/tool/assignments/_assignments.scss @@ -688,9 +688,10 @@ div:first-child { margin-right: 7px; } + } - .expand-icon { - width: 1em; - text-align: center; - } + .expand-icon { + width: 1em; + text-align: center; + } } From 7f82ded99be9aace9c385b3159beb44581c96a24 Mon Sep 17 00:00:00 2001 From: Juan David Massanet Puentes <94039846+JuanDavid102@users.noreply.github.com> Date: Fri, 17 May 2024 14:59:04 +0200 Subject: [PATCH 3/4] SAK-49998 Assignments improve performance and UX in assignments by student --- .../assignment/api/AssignmentService.java | 1 + .../impl/AssignmentServiceImpl.java | 4 ++ .../assignment/tool/AssignmentAction.java | 21 +++--- .../src/webapp/js/assignmentsByStudent.js | 4 +- ...nts_instructor_student_list_submissions.vm | 67 +++++++++---------- 5 files changed, 50 insertions(+), 47 deletions(-) diff --git a/assignment/api/src/java/org/sakaiproject/assignment/api/AssignmentService.java b/assignment/api/src/java/org/sakaiproject/assignment/api/AssignmentService.java index c113affb5b55..7a48dbea5dd6 100644 --- a/assignment/api/src/java/org/sakaiproject/assignment/api/AssignmentService.java +++ b/assignment/api/src/java/org/sakaiproject/assignment/api/AssignmentService.java @@ -530,6 +530,7 @@ public interface AssignmentService extends EntityProducer { public boolean permissionCheck(String permission, String resource, String user); + public boolean permissionCheckInGroups(String permission, Assignment assignment, String user); /** * Access the internal reference which can be used to assess security clearance. * diff --git a/assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java b/assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java index c03ccc015089..b7647a721a42 100644 --- a/assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java +++ b/assignment/impl/src/java/org/sakaiproject/assignment/impl/AssignmentServiceImpl.java @@ -3237,6 +3237,10 @@ public boolean permissionCheck(String permission, String resource, String user) return access; } + public boolean permissionCheckInGroups(String permission, Assignment assignment, String user) { + return this.permissionCheckWithGroups(permission, assignment, user); + } + private boolean permissionCheckWithGroups(final String permission, final Assignment assignment, final String user) { if (StringUtils.isBlank(permission) || assignment == null) return false; String siteReference = siteService.siteReference(assignment.getContext()); diff --git a/assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java b/assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java index b3f491fa093a..21d2e9df7f2c 100644 --- a/assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java +++ b/assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java @@ -1118,6 +1118,7 @@ public class AssignmentAction extends PagedResourceActionII { private static final String CONTEXT_GO_NEXT_UNGRADED_ENABLED = "goNextUngradedEnabled"; private static final String CONTEXT_GO_PREV_UNGRADED_ENABLED = "goPrevUngradedEnabled"; private static final String PARAMS_VIEW_SUBS_ONLY_CHECKBOX = "chkSubsOnly1"; + private static final String EXPANDED_USER_ID = "expandedUserId"; private static ResourceLoader rb = new ResourceLoader("assignment"); private boolean nextUngraded = false; private boolean prevUngraded = false; @@ -5774,25 +5775,19 @@ private String build_instructor_view_students_assignment_context(VelocityPortlet Comparator assignmentComparator = new AssignmentComparator(state, SORTED_BY_DEFAULT, Boolean.TRUE.toString()); Map> showStudentAssignments = new HashMap<>(); - Set expandedStudents = (Set) state.getAttribute(STUDENT_LIST_SHOW_TABLE); if (expandedStudents != null) { context.put("studentListShowSet", expandedStudents); - - List gradableAssignments = assignments.stream() - .filter(a -> assignmentService.allowGradeSubmission(AssignmentReferenceReckoner.reckoner().assignment(a).reckon().getReference())) - .sorted(assignmentComparator) - .collect(Collectors.toList()); - for (String userId : expandedStudents) { - List userSubmittableAssignments = gradableAssignments.stream() - .filter(assignment -> assignmentService.canSubmit(assignment, userId)) - .collect(Collectors.toList()); + Set userSubmittableAssignments = assignments.stream() + .filter(a -> !assignmentService.assignmentUsesAnonymousGrading(a)) + .collect(Collectors.toSet()); showStudentAssignments.put(studentMembers.get(userId), userSubmittableAssignments.iterator()); } } + context.put("expandedUserId", state.getAttribute(EXPANDED_USER_ID)); context.put("studentMembersMap", studentMembers); context.put("studentMembers", new SortedIterator(studentMembers.values().iterator(), new AssignmentComparator(state, SORTED_USER_BY_SORTNAME, Boolean.TRUE.toString()))); context.put("viewGroup", state.getAttribute(VIEW_SUBMISSION_LIST_OPTION)); @@ -11033,7 +11028,9 @@ public void doGrade_submission(RunData data) { String assignmentId = params.getString("assignmentId"); state.setAttribute(EXPORT_ASSIGNMENT_REF, assignmentId); String submissionId = params.getString("submissionId"); - + String userId = params.getString("user_id"); + state.setAttribute(EXPANDED_USER_ID, userId); + // SAK-29314 - put submission information into state boolean viewSubsOnlySelected = stringToBool((String) data.getParameters().getString(PARAMS_VIEW_SUBS_ONLY_CHECKBOX)); putSubmissionInfoIntoState(state, assignmentId, submissionId, viewSubsOnlySelected); @@ -11240,6 +11237,7 @@ public void doShow_student_submission(RunData data) { ParameterParser params = data.getParameters(); String id = params.getString("studentId"); + state.setAttribute(EXPANDED_USER_ID, id); // add the student id into the table t.add(id); @@ -11256,6 +11254,7 @@ public void doHide_student_submission(RunData data) { ParameterParser params = data.getParameters(); String id = params.getString("studentId"); + state.removeAttribute(EXPANDED_USER_ID); // remove the student id from the table t.remove(id); diff --git a/assignment/tool/src/webapp/js/assignmentsByStudent.js b/assignment/tool/src/webapp/js/assignmentsByStudent.js index da53f6e16003..452dfab695cb 100644 --- a/assignment/tool/src/webapp/js/assignmentsByStudent.js +++ b/assignment/tool/src/webapp/js/assignmentsByStudent.js @@ -55,12 +55,12 @@ function parseDataCell(html) { }; } -function renderGrouping({ studentName, actionLink, expanded }) { +function renderGrouping({ studentName, actionLink, expanded, studentUserId }) { const template = document.createElement('template'); template.innerHTML = ` - + ${studentName} diff --git a/assignment/tool/src/webapp/vm/assignment/chef_assignments_instructor_student_list_submissions.vm b/assignment/tool/src/webapp/vm/assignment/chef_assignments_instructor_student_list_submissions.vm index c310189cceb2..d76e29814f80 100644 --- a/assignment/tool/src/webapp/vm/assignment/chef_assignments_instructor_student_list_submissions.vm +++ b/assignment/tool/src/webapp/vm/assignment/chef_assignments_instructor_student_list_submissions.vm @@ -28,6 +28,9 @@ #if (!$!isTimesheet) document.querySelector(".sakai-table-toolBar").classList.add("hidden"); #end + #if ($expandedUserId) + document.getElementById("$expandedUserId").scrollIntoView({ behavior: "instant", block: "center" }); + #end });

@@ -80,7 +83,7 @@ $tlang.getString("gen.student") - + $tlang.getString("gen.assig") @@ -136,10 +139,14 @@ #set($assignments=false) #set($assignments=$!studentAssignmentsTable.get($member)) #foreach ($assignment in $!assignments) + #if ($assignment.isGroup) + #set($displayAssignment = $service.permissionCheckInGroups("asn.submit", $assignment, "$member.id")) + #else + #set($displayAssignment = true) + #end #set ($assignmentReference = $!service.assignmentReference($assignment.Id)) - #set ($isAnon = $!service.assignmentUsesAnonymousGrading($assignment)) #set($assignmentTitle = $formattedText.escapeHtml($assignment.Title)) - #if (!$assignment.Draft) + #if ((!$assignment.Draft) && ($displayAssignment)) ## do not show draft assignments #set($submission = false) #set($submission=$service.getSubmission($assignment.Id, $member)) @@ -158,44 +165,36 @@ - #if (!$isAnon) - $assignmentTitle - #if ($allowAddAssignment && $allowSubmitByInstructor) - #set( $submitSpinnerID = "submitFor_" + $member.Id + "_" + $formattedText.escapeUrl($assignmentReference) ) -
- #if ($assignment.DueDate.isAfter($currentTime)) - - $tlang.getString("submitforstudent") - - #else - $tlang.getString("submitforstudentnotallowed") - #end -
-
+ $assignmentTitle + #if ($allowAddAssignment && $allowSubmitByInstructor) + #set( $submitSpinnerID = "submitFor_" + $member.Id + "_" + $formattedText.escapeUrl($assignmentReference) ) +
+ #if ($assignment.DueDate.isAfter($currentTime)) + + $tlang.getString("submitforstudent") + + #else + $tlang.getString("submitforstudentnotallowed") #end - #else - $assignmentTitle ($tlang.getString("grading.anonymous.title")) +
+
#end #if ($!submission.submitted) - #if (!$isAnon) - #if ($!submission.DateSubmitted) - $!service.getUsersLocalDateTimeString($!submission.DateSubmitted) - #if ($submission.DateSubmitted.isAfter($assignment.DueDate)) - $tlang.getString("gen.late2") - #end + #if ($!submission.DateSubmitted) + $!service.getUsersLocalDateTimeString($!submission.DateSubmitted) + #if ($submission.DateSubmitted.isAfter($assignment.DueDate)) + $tlang.getString("gen.late2") #end - #set ($submissionSubmitter = $!service.getSubmissionSubmittee($submission)) - #if ($!submissionSubmitter.isPresent()) - #set ($submitterId = $!submissionSubmitter.get().getSubmitter()) -
$tlang.getString("listsub.submitted.by") $formattedText.escapeHtml($studentMembersMap.get($submitterId).getDisplayName()) - #if($member.getId() != $submitterId) - ($tlang.getString("listsub.submitted.on.behalf") $formattedText.escapeHtml($member.sortName)) - #end + #end + #set ($submissionSubmitter = $!service.getSubmissionSubmittee($submission)) + #if ($!submissionSubmitter.isPresent()) + #set ($submitterId = $!submissionSubmitter.get().getSubmitter()) +
$tlang.getString("listsub.submitted.by") $formattedText.escapeHtml($studentMembersMap.get($submitterId).getDisplayName()) + #if($member.getId() != $submitterId) + ($tlang.getString("listsub.submitted.on.behalf") $formattedText.escapeHtml($member.sortName)) #end - #elseif ($!submission.DateSubmitted) - $tlang.getString("gen.subm4") $tlang.getString("submitted.date.redacted") #end #end   From bc9bb6888b6c180601b01cc48d0f624046e1fc01 Mon Sep 17 00:00:00 2001 From: Markus Stetschnig Date: Fri, 24 May 2024 19:02:44 +0200 Subject: [PATCH 4/4] SAK-49998 Use method reference --- .../java/org/sakaiproject/assignment/tool/AssignmentAction.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java b/assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java index 21d2e9df7f2c..869c787b8ca3 100644 --- a/assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java +++ b/assignment/tool/src/java/org/sakaiproject/assignment/tool/AssignmentAction.java @@ -5780,7 +5780,7 @@ private String build_instructor_view_students_assignment_context(VelocityPortlet context.put("studentListShowSet", expandedStudents); for (String userId : expandedStudents) { Set userSubmittableAssignments = assignments.stream() - .filter(a -> !assignmentService.assignmentUsesAnonymousGrading(a)) + .filter(Predicate.not(assignmentService::assignmentUsesAnonymousGrading)) .collect(Collectors.toSet()); showStudentAssignments.put(studentMembers.get(userId), userSubmittableAssignments.iterator());