From de7aedac8a4a8c3332c658734385e11253ffef7e Mon Sep 17 00:00:00 2001 From: Adrian Fish Date: Fri, 11 Dec 2020 19:37:03 +0000 Subject: [PATCH] SAK-43138 Display visible status in tool menu (#8900) https://jira.sakaiproject.org/browse/SAK-43138 --- .../java/org/sakaiproject/site/api/Site.java | 6 + .../sakaiproject/tool/api/ToolManager.java | 11 + .../sakaiproject/tool/impl/ToolComponent.java | 30 +++ .../sass/modules/_toolmenu.scss | 49 ++--- .../portal/api/PortalSiteHelper.java | 4 +- .../impl/src/bundle/sitenav.properties | 2 + .../portal/charon/SkinnableCharonPortal.java | 4 - .../charon/handlers/WorksiteHandler.java | 4 +- .../charon/site/PortalSiteHelperImpl.java | 194 +++++++----------- .../src/webapp/vm/morpheus/includePageNav.vm | 148 ++++++------- .../order/impl/SitePageEditHandler.java | 18 +- 11 files changed, 206 insertions(+), 264 deletions(-) diff --git a/kernel/api/src/main/java/org/sakaiproject/site/api/Site.java b/kernel/api/src/main/java/org/sakaiproject/site/api/Site.java index ba9c9805982a..0ff54eee04b3 100644 --- a/kernel/api/src/main/java/org/sakaiproject/site/api/Site.java +++ b/kernel/api/src/main/java/org/sakaiproject/site/api/Site.java @@ -73,6 +73,12 @@ public interface Site extends Edit, Comparable, Serializable, AuthzGroup * property name for custom overview */ public final static String PROP_CUSTOM_OVERVIEW = "custom_overview"; + + /** + * Permission for updating a site + */ + public final static String SITE_UPD = "site.upd"; + /** * @return the user who created this. */ diff --git a/kernel/api/src/main/java/org/sakaiproject/tool/api/ToolManager.java b/kernel/api/src/main/java/org/sakaiproject/tool/api/ToolManager.java index 9c23012226d4..4b0c8cedf8c1 100644 --- a/kernel/api/src/main/java/org/sakaiproject/tool/api/ToolManager.java +++ b/kernel/api/src/main/java/org/sakaiproject/tool/api/ToolManager.java @@ -27,6 +27,7 @@ import java.util.Set; import org.sakaiproject.site.api.Site; +import org.sakaiproject.site.api.SitePage; import org.sakaiproject.site.api.ToolConfiguration; import org.w3c.dom.Document; @@ -119,6 +120,16 @@ public interface ToolManager */ public List> getRequiredPermissions(ToolConfiguration config); + /** + * Tests whether the first tool in the supplied page is visible to ANY + * non-maintainer role. By non-maintainer, we mean a role without + * Site.SITE_UPD. + * + * @param page The site page in which to test the first tool + * @return true, if any role in the site fulfils the required functions of the first tool. false otherwise. + */ + public boolean isFirstToolVisibleToAnyNonMaintainerRole(SitePage page); + /** * Check whether a tool is visible to the current user in this site, * depending on permissions required to view the tool. diff --git a/kernel/kernel-impl/src/main/java/org/sakaiproject/tool/impl/ToolComponent.java b/kernel/kernel-impl/src/main/java/org/sakaiproject/tool/impl/ToolComponent.java index f52e3891683f..9dc084c0e08e 100644 --- a/kernel/kernel-impl/src/main/java/org/sakaiproject/tool/impl/ToolComponent.java +++ b/kernel/kernel-impl/src/main/java/org/sakaiproject/tool/impl/ToolComponent.java @@ -38,6 +38,7 @@ import java.util.Set; import java.util.Vector; import java.util.concurrent.ConcurrentHashMap; +import java.util.stream.Collectors; import lombok.extern.slf4j.Slf4j; @@ -48,8 +49,10 @@ import org.w3c.dom.Node; import org.w3c.dom.NodeList; +import org.sakaiproject.authz.api.Role; import org.sakaiproject.authz.api.SecurityService; import org.sakaiproject.site.api.Site; +import org.sakaiproject.site.api.SitePage; import org.sakaiproject.site.api.ToolConfiguration; import org.sakaiproject.thread_local.api.ThreadLocalManager; import org.sakaiproject.tool.api.Placement; @@ -571,6 +574,33 @@ public List> getRequiredPermissions(ToolConfiguration config) { return sets; } + public boolean isFirstToolVisibleToAnyNonMaintainerRole(SitePage page) { + + List tools = page.getTools(); + List> required + = tools.size() == 1 ? getRequiredPermissions(tools.get(0)) : Collections.EMPTY_LIST; + + if (required.isEmpty()) { + return true; + } + + // Get the non maintainer roles + final Set roles = page.getContainingSite().getRoles().stream() + .filter(r -> !r.isAllowed(Site.SITE_UPD)).collect(Collectors.toSet()); + + // Now check to see if these roles have the permission listed + // in the functions require for the tool. + for (Set permissionSet : required) { + for (Role role : roles) { + if (role.getAllowedFunctions().containsAll(permissionSet)) { + return true; // If any one of the permissions is allows for any role. + } + } + } + + return false; + } + public boolean isVisible(Site site, ToolConfiguration config) { List> permissionSets = getRequiredPermissions(config); diff --git a/library/src/morpheus-master/sass/modules/_toolmenu.scss b/library/src/morpheus-master/sass/modules/_toolmenu.scss index 2a9c5aa4aab1..e6d068518a45 100644 --- a/library/src/morpheus-master/sass/modules/_toolmenu.scss +++ b/library/src/morpheus-master/sass/modules/_toolmenu.scss @@ -94,10 +94,10 @@ body.is-logged-out{ margin: 0; } } + .#{$namespace}toolsNav__menuitem--status-block { + display: none; + } &.is-invisible { - &:after { - display: none; - } &:hover, &:focus { .#{$namespace}toolsNav__menuitem--title{ background: var(--tool-menu-item-hidden-hover-background-color); @@ -500,7 +500,6 @@ nav#subSites{ color: var(--tool-menu-item-icon-color); @if $tool-menu-icon-on-left { - display: inline-block; height: 100%; font-size: 16px; } @else { @@ -513,23 +512,26 @@ nav#subSites{ .#{$namespace}toolsNav__menuitem--title{ @if $tool-menu-icon-on-left { - display: inline-block; width: calc(#{$tool-menu-width} - #{$tool-menu-width-collapsed} - #{$standard-spacing}); vertical-align: middle; margin-left: $standard-spacing; - @media #{$phone}{ - width: 90%; - } - } @else { - display: block; - width: 100%; } text-overflow: ellipsis; white-space: pre; overflow: hidden; } + .#{$namespace}toolsNav__menuitem--status-block { + margin-right: 10px; + margin-left: 4px; + display: flex; + div { + margin-right: 10px; + &.tool-status-icon { + font-size: 16px; + } + } + } &.is-invisible { - position: relative; font-style: italic; color: var(--tool-menu-item-hidden-text-color); background: var(--tool-menu-item-hidden-background-color); @@ -538,20 +540,6 @@ nav#subSites{ border-left: $tool-menu-item-hidden-border-left; } - &:after { - @extend .fa-lg; - @extend .fa; - content: '\f070'; - @if $tool-menu-icon-on-left { - margin-right: $standard-spacing; - } - @else { - position: absolute; - top: 4px; - right: 4px; - } - } - &:hover{ color: var(--tool-menu-item-hidden-hover-text-color); background: var(--tool-menu-item-hidden-hover-background-color); @@ -934,15 +922,6 @@ body { color: var(--tool-menu-item-hidden-hover-icon-color) !important; } } - - &:after { - @extend .fa-lg; - @extend .fa; - content: '\f070'; - position: absolute; - top: 8px; - right: 4px; - } } &.has-prerequisite { color: var(--tool-menu-item-hidden-text-color) !important; diff --git a/portal/portal-api/api/src/java/org/sakaiproject/portal/api/PortalSiteHelper.java b/portal/portal-api/api/src/java/org/sakaiproject/portal/api/PortalSiteHelper.java index a6d00c9e0830..c6d738e7d323 100644 --- a/portal/portal-api/api/src/java/org/sakaiproject/portal/api/PortalSiteHelper.java +++ b/portal/portal-api/api/src/java/org/sakaiproject/portal/api/PortalSiteHelper.java @@ -92,11 +92,11 @@ public interface PortalSiteHelper * @param resetTools * @param includeSummary */ - Map pageListToMap(HttpServletRequest req, boolean loggedIn, Site site, + Map pageListToMap(HttpServletRequest req, boolean loggedIn, Site site, SitePage page, String toolContextPath, String portalPrefix, boolean doPages, boolean resetTools, boolean includeSummary); - Map convertSiteToMap(HttpServletRequest req, Site s, String prefix, + Map convertSiteToMap(HttpServletRequest req, Site s, String prefix, String currentSiteId, String myWorkspaceSiteId, boolean includeSummary, boolean expandSite, boolean resetTools, boolean doPages, String toolContextPath, boolean loggedIn, List siteProviders); diff --git a/portal/portal-impl/impl/src/bundle/sitenav.properties b/portal/portal-impl/impl/src/bundle/sitenav.properties index 06a3731f8c40..9b5e8823211d 100644 --- a/portal/portal-impl/impl/src/bundle/sitenav.properties +++ b/portal/portal-impl/impl/src/bundle/sitenav.properties @@ -81,6 +81,8 @@ sit_no_favorites_selected=You do not have any favorite sites. Star sites in the sit_autofav_description=Automatically add new sites to your favorites bar: sit_favorite_limit_reached=Only the first {0} sites (above) will display in your favorites bar. sit_open_menu=Open attached menu for {0} to access its tools +site_tool_locked_tooltip=This tool is not available to non admin users +site_tool_hidden_tooltip=This tool is not visible for non admin users site_fav_toggle=Toggle {0} as a favorite site sit_manover = Manage Overview diff --git a/portal/portal-impl/impl/src/java/org/sakaiproject/portal/charon/SkinnableCharonPortal.java b/portal/portal-impl/impl/src/java/org/sakaiproject/portal/charon/SkinnableCharonPortal.java index 36f3430a01fa..8d6ce8e7e405 100644 --- a/portal/portal-impl/impl/src/java/org/sakaiproject/portal/charon/SkinnableCharonPortal.java +++ b/portal/portal-impl/impl/src/java/org/sakaiproject/portal/charon/SkinnableCharonPortal.java @@ -561,10 +561,6 @@ public PortalRenderContext includePortal(HttpServletRequest req, } } - //List l = siteHelper.convertSitesToMaps(req, mySites, prefix, siteId, myWorkspaceSiteId, - // includeSummary, expandSite, resetTools, doPages, toolContextPath, - // loggedIn); - SiteView siteView = siteHelper.getSitesView(SiteView.View.ALL_SITES_VIEW, req, session, siteId ); siteView.setPrefix(prefix); siteView.setResetTools(resetTools); diff --git a/portal/portal-impl/impl/src/java/org/sakaiproject/portal/charon/handlers/WorksiteHandler.java b/portal/portal-impl/impl/src/java/org/sakaiproject/portal/charon/handlers/WorksiteHandler.java index ab806842fc9e..e790b989243a 100644 --- a/portal/portal-impl/impl/src/java/org/sakaiproject/portal/charon/handlers/WorksiteHandler.java +++ b/portal/portal-impl/impl/src/java/org/sakaiproject/portal/charon/handlers/WorksiteHandler.java @@ -188,8 +188,8 @@ public void includeWorksite(PortalRenderContext rcontext, HttpServletResponse re { if (rcontext.uses(INCLUDE_PAGE_NAV)) { - boolean loggedIn = session.getUserId() != null; - Map pageMap = portal.getSiteHelper().pageListToMap(req, loggedIn, site, page, toolContextPath, + boolean loggedIn = session.getUserId() != null; + Map pageMap = portal.getSiteHelper().pageListToMap(req, loggedIn, site, page, toolContextPath, portalPrefix, /* doPages */true, /* resetTools */"true".equalsIgnoreCase(ServerConfigurationService diff --git a/portal/portal-impl/impl/src/java/org/sakaiproject/portal/charon/site/PortalSiteHelperImpl.java b/portal/portal-impl/impl/src/java/org/sakaiproject/portal/charon/site/PortalSiteHelperImpl.java index 48e82c3bc08f..efd848376898 100644 --- a/portal/portal-impl/impl/src/java/org/sakaiproject/portal/charon/site/PortalSiteHelperImpl.java +++ b/portal/portal-impl/impl/src/java/org/sakaiproject/portal/charon/site/PortalSiteHelperImpl.java @@ -32,9 +32,10 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Properties; import java.util.Set; -import java.util.Vector; +import java.util.stream.Collectors; import javax.servlet.http.HttpServletRequest; @@ -184,8 +185,7 @@ public boolean doGatewaySiteList() */ public String getGatewaySiteId() { - String gatewaySiteListPref = ServerConfigurationService - .getString("gatewaySiteList"); + String gatewaySiteListPref = ServerConfigurationService.getString("gatewaySiteList"); if (gatewaySiteListPref == null) return null; @@ -196,10 +196,8 @@ public String getGatewaySiteId() } // Loop throught the sites making sure they exist and are visitable - for (int i = 0; i < gatewaySiteIds.length; i++) + for (String siteId : gatewaySiteIds) { - String siteId = gatewaySiteIds[i]; - Site site = null; try { @@ -220,8 +218,7 @@ public String getGatewaySiteId() } } - log.warn("No suitable gateway sites found, gatewaySiteList preference had " - + gatewaySiteIds.length + " sites."); + log.warn("No suitable gateway sites found, gatewaySiteList preference had {} sites", gatewaySiteIds.length); return null; } @@ -234,15 +231,15 @@ public String getGatewaySiteId() */ private String[] getGatewaySiteList() { - String gatewaySiteListPref = ServerConfigurationService - .getString("gatewaySiteList"); + String gatewaySiteListPref = ServerConfigurationService.getString("gatewaySiteList"); if (gatewaySiteListPref == null || gatewaySiteListPref.trim().length() < 1) { gatewaySiteListPref = ServerConfigurationService.getGatewaySiteId(); } - if (gatewaySiteListPref == null || gatewaySiteListPref.trim().length() < 1) + if (gatewaySiteListPref == null || gatewaySiteListPref.trim().length() < 1) { return null; + } String[] gatewaySites = gatewaySiteListPref.split(","); if (gatewaySites.length < 1) return null; @@ -262,7 +259,7 @@ private String[] getGatewaySiteList() public List getSubSites(Site site) { if (site == null) return null; - Map propMap = new HashMap(); + Map propMap = new HashMap<>(); propMap.put(PROP_PARENT_ID, site.getId()); // This should not call getUserSites(boolean) because the property is variable, while the call is cacheable otherwise @@ -274,7 +271,6 @@ public List getSubSites(Site site) public List getSitesInContext(String context, String userId) { - return null; } @@ -288,13 +284,13 @@ public List getSitesInContext(String context, String userId) * java.lang.String, boolean, boolean, boolean, boolean, * java.lang.String, boolean) */ - public List convertSitesToMaps(HttpServletRequest req, List mySites, + public List convertSitesToMaps(HttpServletRequest req, List mySites, String prefix, String currentSiteId, String myWorkspaceSiteId, boolean includeSummary, boolean expandSite, boolean resetTools, boolean doPages, String toolContextPath, boolean loggedIn) { - List l = new ArrayList(); - Map depthChart = new HashMap(); + List l = new ArrayList<>(); + Map depthChart = new HashMap<>(); boolean motdDone = false; // We only compute the depths if there is no user chosen order @@ -303,25 +299,22 @@ public List convertSitesToMaps(HttpServletRequest req, List mySites, List favorites = Collections.emptyList(); - if ( session != null ) - { - Preferences prefs = PreferencesService.getPreferences(session.getUserId()); - ResourceProperties props = prefs.getProperties(org.sakaiproject.user.api.PreferencesService.SITENAV_PREFS_KEY); + if ( session != null ) { + Preferences prefs = PreferencesService.getPreferences(session.getUserId()); + ResourceProperties props = prefs.getProperties(org.sakaiproject.user.api.PreferencesService.SITENAV_PREFS_KEY); - List propList = props.getPropertyList("order"); - if (propList != null) - { - computeDepth = false; - favorites = propList; - } - } + List propList = props.getPropertyList("order"); + if (propList != null) + { + computeDepth = false; + favorites = propList; + } + } // Determine the depths of the child sites if needed Map> realmProviderMap = getProviderIDsForSites(mySites); - for (Iterator i = mySites.iterator(); i.hasNext();) + for (Site s : mySites) { - Site s = (Site) i.next(); - // The first site is the current site if (currentSiteId == null) currentSiteId = s.getId(); @@ -343,7 +336,7 @@ public List convertSitesToMaps(HttpServletRequest req, List mySites, log.debug("Depth = {}", cDepth); } - Map m = convertSiteToMap(req, s, prefix, currentSiteId, myWorkspaceSiteId, + Map m = convertSiteToMap(req, s, prefix, currentSiteId, myWorkspaceSiteId, includeSummary, expandSite, resetTools, doPages, toolContextPath, loggedIn, realmProviderMap.get(s.getReference())); @@ -394,21 +387,16 @@ public static List getProviderIDsForSite(Site site) * @param sites the list of sites to retrieve all provider IDs * @return a Map, where the key is the realm ID, and the value is a list of provider IDs for that site */ - public static Map> getProviderIDsForSites(List sites) - { - Map> realmProviderMap = new HashMap<>(); - if (!sites.isEmpty()) - { - List realmIDs = new ArrayList<>(); - for (Site site : sites) - { - realmIDs.add(site.getReference()); - } + public static Map> getProviderIDsForSites(List sites) { - realmProviderMap = getAuthzGroupService().getProviderIDsForRealms(realmIDs); + if (sites.isEmpty()) { + return Collections.EMPTY_MAP; } - return realmProviderMap; + List realmIDs + = sites.stream().map(s -> s.getReference()).collect(Collectors.toList()); + + return getAuthzGroupService().getProviderIDsForRealms(realmIDs); } /** @@ -448,7 +436,7 @@ public String getUserSpecificSiteTitle(Site site, boolean truncated, boolean esc * java.lang.String, boolean, boolean, boolean, boolean, * java.lang.String, boolean, java.util.List) */ - public Map convertSiteToMap(HttpServletRequest req, Site s, String prefix, + public Map convertSiteToMap(HttpServletRequest req, Site s, String prefix, String currentSiteId, String myWorkspaceSiteId, boolean includeSummary, boolean expandSite, boolean resetTools, boolean doPages, String toolContextPath, boolean loggedIn, List siteProviders) @@ -477,7 +465,7 @@ public Map convertSiteToMap(HttpServletRequest req, Site s, String prefix, m.put("siteDescription", s.getHtmlDescription()); - if ( s.getShortDescription() !=null && s.getShortDescription().trim().length()>0 ){ + if (s.getShortDescription() != null && s.getShortDescription().trim().length() > 0) { // SAK-23895: Allow display of site description in the tab instead of site title String shortDesc = s.getShortDescription(); String shortDesc_trimmed = getFormattedText().makeShortenedText(shortDesc, null, null, null); @@ -562,8 +550,8 @@ private List getPwd(Site s, String ourParent) log.debug("Getting Current Working Directory for {} {}", s.getId(), s.getTitle()); int depth = 0; - Vector pwd = new Vector(); - Set added = new HashSet(); + List pwd = new ArrayList<>(); + Set added = new HashSet<>(); // Add us to the list at the top (will become the end) pwd.add(s); @@ -586,7 +574,7 @@ private List getPwd(Site s, String ourParent) if (added.contains(site.getId())) break; log.debug("Adding Parent {} {}", site.getId(), site.getTitle()); - pwd.insertElementAt(site, 0); // Push down stack + pwd.add(0, site); // Push down stack added.add(site.getId()); ResourceProperties rp = site.getProperties(); @@ -610,12 +598,12 @@ private List getPwd(Site s, String ourParent) * org.sakaiproject.site.api.SitePage, java.lang.String, * java.lang.String, boolean, boolean, boolean) */ - public Map pageListToMap(HttpServletRequest req, boolean loggedIn, Site site, + public Map pageListToMap(HttpServletRequest req, boolean loggedIn, Site site, SitePage page, String toolContextPath, String portalPrefix, boolean doPages, boolean resetTools, boolean includeSummary) { - Map theMap = new HashMap(); + Map theMap = new HashMap<>(); String effectiveSiteId = getSiteEffectiveId(site); @@ -644,8 +632,9 @@ else if ("always".equals(showHelpGlobal)) String iconUrl = ""; try { - if (site.getIconUrlFull() != null) + if (site.getIconUrlFull() != null) { iconUrl = new URI(site.getIconUrlFull()).toString(); + } } catch (URISyntaxException uex) { log.debug("Icon URL is invalid: " + site.getIconUrlFull()); } @@ -662,37 +651,21 @@ else if ("always".equals(showHelpGlobal)) boolean siteUpdate = SecurityService.unlock("site.upd", site.getReference()); - // theMap.put("pageNavSitToolsHead", - // Web.escapeHtml(rb.getString("sit_toolshead"))); - - // order the pages based on their tools and the tool order for the - // site type - // List pages = site.getOrderedPages(); - List pages = getPermittedPagesInOrder(site); - - List l = new ArrayList(); + List l = new ArrayList<>(); String addMoreToolsUrl = null; String manageOverviewUrl = null; - for (Iterator i = pages.iterator(); i.hasNext();) - { - - SitePage p = (SitePage) i.next(); + for (SitePage p : getPermittedPagesInOrder(site)) { // check if current user has permission to see page // one tool on the page - List pTools = p.getTools(); + List pageTools = p.getTools(); ToolConfiguration firstTool = null; String toolsOnPage = null; // Check the tools that indicate the portal is to do the popup - Iterator toolz = pTools.iterator(); String source = null; - int count = 0; - ToolConfiguration pageTool = null; - while(toolz.hasNext()){ - count++; - pageTool = toolz.next(); + for (ToolConfiguration pageTool : pageTools) { source = ToolUtils.getToolPopupUrl(pageTool); if ( "sakai.siteinfo".equals(pageTool.getToolId()) ) { addMoreToolsUrl = ToolUtils.getPageUrl(req, site, p, portalPrefix, @@ -703,43 +676,34 @@ else if ("always".equals(showHelpGlobal)) manageOverviewUrl += "?sakai_action=doManageOverviewFromHome"; } } - if ( count != 1 ) { + if ( pageTools.size() != 1 ) { source = null; addMoreToolsUrl = null; - pageTool = null; } - boolean current = (page != null && p.getId().equals(page.getId()) && !p - .isPopUp()); + boolean current = (page != null && p.getId().equals(page.getId()) && !p.isPopUp()); String pageAlias = lookupPageToAlias(site.getId(), p); String pagerefUrl = ToolUtils.getPageUrl(req, site, p, portalPrefix, resetTools, effectiveSiteId, pageAlias); if (doPages || p.isPopUp()) { - Map m = new HashMap(); - StringBuffer desc = new StringBuffer(); + Map m = new HashMap<>(); + String desc = new String(); boolean hidden = false; - if (pTools != null && pTools.size() > 0) { - firstTool = pTools.get(0); + if (pageTools != null && pageTools.size() > 0) { + firstTool = pageTools.get(0); hidden = true; // Only set the page to hidden when we have tools that might un-hide it. - Iterator tools = pTools.iterator(); //get the tool descriptions for this page, typically only one per page, execpt for the Home page - int tCount = 0; - while(tools.hasNext()){ - ToolConfiguration t = tools.next(); - if (hidden && !isHidden(t)) - { - hidden = false; + for (ToolConfiguration tc : pageTools) { + if (hidden && !isHidden(tc)) { + hidden = false; } - if (tCount > 0){ - desc.append(" | "); - } - if ( t.getTool() == null ) continue; - desc.append(t.getTool().getDescription()); - tCount++; } + desc = String.join(" | ", pageTools.stream().map(tc -> tc.getTool()) + .filter(Objects::nonNull) + .map(t -> t.getDescription()).collect(Collectors.toList())); } if ( ! siteUpdate ){ @@ -775,9 +739,11 @@ else if ("always".equals(showHelpGlobal)) m.put("toolpopupurl", source); // TODO: Should have Web.escapeHtmlAttribute() - String description = desc.toString().replace("\"","""); + String description = desc.replace("\"","""); m.put("description", description); m.put("hidden", Boolean.valueOf(hidden)); + boolean locked = !toolManager.isFirstToolVisibleToAnyNonMaintainerRole(p); + m.put("locked", Boolean.valueOf(locked)); // toolsOnPage is always null //if (toolsOnPage != null) m.put("toolsOnPage", toolsOnPage); if (includeSummary) summarizePage(m, site, p); @@ -818,7 +784,7 @@ else if ("always".equals(showHelpGlobal)) } // Loop through the tools again and Unroll the tools - Iterator iPt = pTools.iterator(); + Iterator iPt = pageTools.iterator(); while (iPt.hasNext()) { @@ -909,18 +875,18 @@ else if ("always".equals(showHelpGlobal)) } else { showPresence = Boolean.valueOf(globalShowPresence).booleanValue(); String showPresenceSite = site.getProperties().getProperty("display-users-present"); - if (showPresenceSite != null) - { + if (showPresenceSite != null) { showPresence = Boolean.valueOf(showPresenceSite).booleanValue(); } } // Check to see if this is a my workspace site, and if so, whether presence is disabled - if (showPresence && SiteService.isUserSite(site.getId()) && !ServerConfigurationService.getBoolean("display.users.present.myworkspace", false)) + if (showPresence && SiteService.isUserSite(site.getId()) && !ServerConfigurationService.getBoolean("display.users.present.myworkspace", false)) { showPresence = false; + } - String presenceUrl = Web.returnUrl(req, "/presence/" - + getFormattedText().escapeUrl(site.getId())); + String presenceUrl + = Web.returnUrl(req, "/presence/" + getFormattedText().escapeUrl(site.getId())); theMap.put("pageNavShowPresenceLoggedIn", Boolean.valueOf(showPresence && loggedIn)); @@ -1036,8 +1002,8 @@ public boolean setTemporaryPlacement(Site site) public boolean summarizePage(Map m, Site site, SitePage page) { - List pTools = page.getTools(); - Iterator iPt = pTools.iterator(); + List pageTools = page.getTools(); + Iterator iPt = pageTools.iterator(); while (iPt.hasNext()) { ToolConfiguration placement = (ToolConfiguration) iPt.next(); @@ -1066,7 +1032,7 @@ private boolean summarizeTool(Map m, Site site, String toolIdentifier) if (site == null) return false; setTemporaryPlacement(site); - Map newMap = null; + Map newMap = null; /* * This is a new, cooler way to do this (I hope) chmaurer... (ieb) Yes:) @@ -1074,9 +1040,8 @@ private boolean summarizeTool(Map m, Site site, String toolIdentifier) */ // offer to all EntityProducers - for (Iterator i = EntityManager.getEntityProducers().iterator(); i.hasNext();) + for (EntityProducer ep : EntityManager.getEntityProducers()) { - EntityProducer ep = (EntityProducer) i.next(); if (ep instanceof EntitySummary) { try @@ -1094,8 +1059,8 @@ private boolean summarizeTool(Map m, Site site, String toolIdentifier) catch (Throwable t) { log.warn( - "Error encountered while asking EntitySummary to getSummary() for: " - + toolIdentifier, t); + "Error encountered while asking EntitySummary to getSummary() for: " + + toolIdentifier, t); } } } @@ -1209,8 +1174,6 @@ public Site getSiteVisit(String siteId) throws PermissionException, IdUnusedExce } } - - /** * Retrieve the list of pages in this site, checking to see if the user has * permission to see the page - by checking the permissions of tools on the @@ -1219,7 +1182,7 @@ public Site getSiteVisit(String siteId) throws PermissionException, IdUnusedExce * @param site * @return */ - public List getPermittedPagesInOrder(Site site) + protected List getPermittedPagesInOrder(Site site) { // Get all of the pages List pages = site.getOrderedPages(); @@ -1230,15 +1193,10 @@ public List getPermittedPagesInOrder(Site site) for (SitePage p : pages) { // check if current user has permission to see page - List pTools = p.getTools(); - Iterator iPt = pTools.iterator(); boolean allowPage = false; - while (iPt.hasNext()) - { - ToolConfiguration placement = (ToolConfiguration) iPt.next(); - - boolean thisTool = allowTool(site, placement); - boolean unHidden = siteUpdate || ! isHidden(placement); + for (ToolConfiguration tc : p.getTools()) { + boolean thisTool = allowTool(site, tc); + boolean unHidden = siteUpdate || ! isHidden(tc); if (thisTool && unHidden) allowPage = true; } if (allowPage) newPages.add(p); @@ -1287,7 +1245,7 @@ public List getPermittedPagesInOrder(Site site) public SitePage lookupSitePage(String pageId, Site site) { // Make sure we have some permitted pages - List pages = getPermittedPagesInOrder(site); + List pages = getPermittedPagesInOrder(site); if (pages.isEmpty()) return null; SitePage page = site.getPage(pageId); if (page == null) diff --git a/portal/portal-render-engine-impl/impl/src/webapp/vm/morpheus/includePageNav.vm b/portal/portal-render-engine-impl/impl/src/webapp/vm/morpheus/includePageNav.vm index d73ed7326341..8c5dd1d0ca9a 100644 --- a/portal/portal-render-engine-impl/impl/src/webapp/vm/morpheus/includePageNav.vm +++ b/portal/portal-render-engine-impl/impl/src/webapp/vm/morpheus/includePageNav.vm @@ -1,5 +1,47 @@ +#macro( toolLink $url $current $popupUrl $subsite) +
  • + +
    + #if ($subsite) +
    ${rloader.subsite} ${site.siteTitle}
    + #else +
    ${page.pageTitle}
    + #{end} +
    + #if (${page.hidden}) +
    + #{end} + #if (${page.locked}) +
    + #{end} +
    +     +
    • +#end +
    #if ($sitePages.siteHTMLInclude) @@ -33,49 +75,17 @@ #else #if (${page.current}) #set( $breadToolName = $page.pageTitle ) - -
  • - - - ${page.pageTitle} - -
    • - + #toolLink(${page.pageResetUrl}, true, false) #else - #if (${page.toolpopup}) - -
  • - - - ${page.pageTitle} - -
    • - + #toolLink("", false, "${page.toolpopupurl}") #else - #if (${page.ispopup}) - -
  • - - - ${page.pageTitle} - -
    • - + #toolLink("", false, "${page.pagePopupUrl}${page.pageId}") #else - -
  • - - - ${page.pageTitle} - -
    • - + #toolLink(${page.pageRefUrl}, false, false) #end ## END of IF (${page.ispopup}) - #end ## END of IF (${page.toolpopup}) - #end ## END of IF (${page.current}) #end ## END of Else @@ -88,51 +98,18 @@ #foreach( $page in $sitePages.pageNavTools ) #if (${page.pageProps.get("sitePage.pageCategory")}==$key) #if (${page.current}) - -
  • - - - ${page.pageTitle} - -
    • - + #toolLink(${page.pageResetUrl}, true, false) #else - #if (${page.toolpopup}) - -
  • - - - ${page.pageTitle} - -
    • - + #toolLink("", false, "${page.toolpopupurl}") #else - #if (${page.ispopup}) - -
  • - - - ${page.pageTitle} - -
    • - + #toolLink("", false, "${page.pagePopupUrl}${page.pageId}") #else - -
  • - - - ${page.pageTitle} - -
    • - + #toolLink(${page.pageRefUrl}, false, false) #end ## END of IF (${page.ispopup}) - #end ## END of IF (${page.toolpopup}) - #end ## END of IF (${page.current}) - #end ## End if key matched category #end ## End for each page @@ -143,8 +120,8 @@
  • - - ${rloader.subsites} +
    +
    ${rloader.subsites}
    • @@ -164,9 +141,15 @@ #if (${sitePages.pageNavShowHelp})
  • - - - ${rloader.sit_help} + +
    +
    ${rloader.sit_help}
    ${rloader.site_newwindow}
    • @@ -188,14 +171,7 @@ diff --git a/site-manage/pageorder/tool/src/java/org/sakaiproject/site/tool/helper/order/impl/SitePageEditHandler.java b/site-manage/pageorder/tool/src/java/org/sakaiproject/site/tool/helper/order/impl/SitePageEditHandler.java index 4c8fcac82395..915059ca6331 100644 --- a/site-manage/pageorder/tool/src/java/org/sakaiproject/site/tool/helper/order/impl/SitePageEditHandler.java +++ b/site-manage/pageorder/tool/src/java/org/sakaiproject/site/tool/helper/order/impl/SitePageEditHandler.java @@ -16,10 +16,8 @@ package org.sakaiproject.site.tool.helper.order.impl; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; -import java.util.Comparator; import java.util.HashSet; import java.util.Iterator; import java.util.LinkedHashMap; @@ -29,7 +27,6 @@ import java.util.Set; import java.util.stream.Collectors; -import org.apache.commons.lang3.StringUtils; import org.sakaiproject.authz.api.AuthzGroup; import org.sakaiproject.authz.api.AuthzGroupService; import org.sakaiproject.authz.api.AuthzPermissionException; @@ -437,20 +434,7 @@ public boolean isVisible(SitePage page) { * @return true if users with out site.upd can see the page */ public boolean isEnabled(SitePage page) { - - // Grab the non site.upd roles (ie : non maintainer types) - Set roles = getRolesWithout(page.getContainingSite(), SITE_UPD); - - // Now check to see if these roles have the permission listed - // in the functions require for the tool. - for (Set permissionSet : getSingleToolPagePermissions(page)) { - for (Role role : roles) { - if (role.getAllowedFunctions().containsAll(permissionSet)) { - return true; // If any one of the permissions is allows for any role. - } - } - } - return false; + return toolManager.isFirstToolVisibleToAnyNonMaintainerRole(page); } /**