SAK-30656 - Add delete tool checkboxes to delete deployment screen (s…

…akaiproject#4617)

basiclti/basiclti-api/src/java/org/sakaiproject/lti/api/LTIService.java

@@ -323,6 +323,8 @@ public interface LTIService {
 
     boolean deleteTool(Long key, String siteId);
 
+    public List<String>  deleteToolAndContents(Long key, String siteId);
+
     boolean deleteToolDao(Long key, String siteId, boolean isAdminRole, boolean isMaintainRole);
 
     Map<String, Object> getTool(Long key, String siteId);

basiclti/basiclti-common/src/java/org/sakaiproject/util/foorm/Foorm.java

@@ -1460,6 +1460,10 @@ public String searchCheck(String search, String tableName, String[] fieldinfo) {
 		if (search == null) {
 			return null;
 		}
+		//check if is a direct search
+		if (StringUtils.isNotEmpty(search) && search.matches("(\\w+\\.)?\\w+\\s*=.+")) {
+			return search;
+		}
 		StringBuilder sb = new StringBuilder();
 		List<String> tokens = getSearchTokens(search);
 		List<String> separators = getSearchSeparators(search);

basiclti/basiclti-common/src/test/org/sakaiproject/util/foorm/TestFoormJUnit.java

@@ -117,8 +117,34 @@ public void testBasicFormExtraction() {
 	@Test
 	public void testSearchCheckSql() {
 		Foorm foorm = new Foorm();
-		// There was a problem with a NPE being thrown when there weren't any colons in the search
-		assertEquals(null, foorm.searchCheck("COLUMN = 'value'", "table", new String[]{}));
+		// SAK-32704 - Want to review all this can perhaps adjust how the decision is made
+		// about which search approach is in use
+		String whereStyle = "COLUMN = 'value'";
+		assertEquals(whereStyle, foorm.searchCheck(whereStyle, "table", new String[]{}));
+		whereStyle = "COLUMN='value'";
+		assertEquals(whereStyle, foorm.searchCheck(whereStyle, "table", new String[]{}));
+		whereStyle = "COLUMN='value' OR OTHER=42";
+		assertEquals(whereStyle,foorm.searchCheck(whereStyle, "table", new String[]{}));
+		whereStyle = "COLUMN='value' OR ( OTHER=42 and zap=21)";
+		assertEquals(whereStyle, foorm.searchCheck(whereStyle, "table", new String[]{}));
+
+		// Workaround for the Null ones below - not pretty but works
+		whereStyle = "1=1 and (table.COLUMN LIKE '%zap%' OR ( othertable.OTHER=42 and zap=21))";
+		assertEquals(whereStyle, foorm.searchCheck(whereStyle, "table", new String[]{}));
+
+		// At some point we might want these to pass
+		whereStyle = "(1=1) and (table.COLUMN LIKE '%zap%' OR ( othertable.OTHER=42 and zap=21))";
+		assertNull(foorm.searchCheck(whereStyle, "table", new String[]{}));
+		whereStyle = "table.COLUMN LIKE '%zap%' OR ( othertable.OTHER=42 and zap=21)";
+		assertNull(foorm.searchCheck(whereStyle, "table", new String[]{}));
+
+		// A parenthesis first should be OK
+		whereStyle = "(COLUMN='value') OR ( OTHER=42 and zap=21)";
+		assertNull(foorm.searchCheck(whereStyle, "table", new String[]{}));
+		whereStyle = "( COLUMN = 'value' ) OR ( OTHER=42 and zap=21)";
+		assertNull(foorm.searchCheck(whereStyle, "table", new String[]{}));
+		whereStyle = " ( COLUMN = 'value' ) OR ( OTHER=42 and zap=21)";
+		assertNull(foorm.searchCheck(whereStyle, "table", new String[]{}));
 	}
 
 	@Test

basiclti/basiclti-impl/src/java/org/sakaiproject/lti/impl/BaseLTIService.java

@@ -445,6 +445,73 @@ public boolean deleteTool(Long key, String siteId) {
 		return deleteToolDao(key, siteId, isAdmin(siteId), isMaintain(siteId));
 	}
 
+	/** Delete a tool and delete the content items and site links assocated with the tool
+	 *
+	 * This is called by a maintain user in a regualr site and deletes a tool, its content
+	 * item, and pages with it on the page.
+	 *
+	 * For the admin user in the !admin site - it deletes a tool and then removes the
+	 * placements + pages from all the sites that have the tool - might take a second or two.
+	 *
+	 * @return A list of strings that are error messages
+	 */
+	@Override
+	public List<String> deleteToolAndContents(Long key, String siteId) {
+
+		List<String> retval = new ArrayList<String> ();
+		String errstr;
+
+		List<Map<String,Object>> contents;
+		if ( isAdmin(siteId) ) {
+			contents = this.getContentsDao("lti_content.tool_id = "+key,null,0,5000, null, isAdmin(siteId));
+		} else {
+			contents = this.getContents("lti_content.tool_id = "+key, null,0,5000, siteId);
+		}
+
+		for ( Map<String,Object> content : contents ) {
+			// the content with same tool id remove the content link first
+			Long content_key = foorm.getLongNull(content.get(LTIService.LTI_ID));
+			if ( content_key == null ) continue;
+
+			// Check the tool_id - just double checking in case the WHER clause fails
+			Long tool_id = foorm.getLongNull(content.get(LTIService.LTI_TOOL_ID));
+			if ( ! key.equals(tool_id) ) continue;
+
+			// Admin edits all sites with the content item
+			String contentSite = siteId;
+			if ( isAdmin(siteId) ) {
+				contentSite = content.get(LTIService.LTI_SITE_ID).toString();
+			}
+
+			// Is there is a tool placement in the left Nav (i.e. not Lessons)
+			// remove the tool content link page from the site
+			String pstr = (String) content.get(LTIService.LTI_PLACEMENT);
+			if ( pstr != null && pstr.length() > 1 ) {
+				errstr = this.deleteContentLink(content_key, contentSite);
+				if ( errstr != null ) {
+					M_log.debug(errstr);
+					retval.add(errstr);
+				}
+			}
+
+			// remove the content item that depends on the tool
+			if ( ! this.deleteContent(content_key, contentSite) ) {
+				errstr = "Unable to delete content itemkey="+key+" site="+siteId;
+				M_log.debug(errstr);
+				retval.add(errstr);
+			}
+		}
+
+		// We are going to delete the tool even if there were problems along the way
+		// Since that is the one thing we are supposed to do in this method
+		if ( ! this.deleteTool(key, siteId) ) {
+			errstr = "Unable to delete tool key="+key+" site="+siteId;
+			M_log.debug(errstr);
+			retval.add(errstr);
+		}
+		return retval;
+	}
+
 	@Override
 	public Map<String, Object> getTool(Long key, String siteId) {
 		return getToolDao(key, siteId, isAdmin(siteId));
@@ -656,14 +723,14 @@ protected Object insertToolSiteLinkDao(String id, String button_text, String sit
 			catch (PermissionException ee)
 			{
 				retval = new String("0" + rb.getFormattedMessage("error.link.placement.update", new Object[]{id}));
-				M_log.warn(this + " cannot add page and basic lti tool to site " + siteId);
+				M_log.warn("Cannot add page and basic lti tool to site " + siteId);
 			}
 		}
 		catch (IdUnusedException e)
 		{
 			// cannot find site
 			retval = new String("0" + rb.getFormattedMessage("error.link.placement.update", new Object[]{id}));
-			M_log.warn(this + " cannot find site " + contentSite);
+			M_log.warn("Cannot find site " + contentSite);
 		}
 
 		return retval;
@@ -700,7 +767,7 @@ protected String deleteContentLinkDao(Long key, String siteId, boolean isAdminRo
 		}
 
 		String siteStr = (String) content.get(LTI_SITE_ID);
-		// only admin can remve content from other site
+		// only admin can remove content from other site
 		if ( ! siteId.equals(siteStr) && !isAdminRole ) {
 			return rb.getString("error.placement.not.found");
 		}
@@ -719,10 +786,11 @@ protected String deleteContentLinkDao(Long key, String siteId, boolean isAdminRo
 					return rb.getString("error.placement.not.removed");
 				}
 			} else {
-				M_log.warn(this + " LTI content="+key+" placement="+tool.getId()+" could not find page in site=" + siteStr);
+				M_log.warn("LTI content="+key+" placement="+tool.getId()+" could not find page in site=" + siteStr);
 			}
 
 			// Remove the placement from the content item
+			// Our caller can remove the contentitem if they like
 			Properties newProps = new Properties();
 			newProps.setProperty(LTIService.LTI_PLACEMENT, "");
 			Object retval = updateContentDao(key, newProps, siteId, isAdminRole, isMaintainRole);
@@ -736,7 +804,7 @@ protected String deleteContentLinkDao(Long key, String siteId, boolean isAdminRo
 		}
 		catch (IdUnusedException ee)
 		{
-			M_log.warn(this + " LTI content="+key+" placement="+tool.getId()+" could not remove page from site=" + siteStr);
+			M_log.warn("LTI content="+key+" placement="+tool.getId()+" could not remove page from site=" + siteStr);
 			return new String(rb.getFormattedMessage("error.link.placement.update", new Object[]{key.toString()}));
 		}
 	}

basiclti/basiclti-impl/src/java/org/sakaiproject/lti/impl/DBLTIService.java

@@ -415,31 +415,39 @@ public Object updateContentDao(Long key, Object newProps, String siteId,
 			key, newProps, siteId, isAdminRole, isMaintainRole);
 	}
 
+	/**
+	 * Get the contents for a search, add some data from site properties, and the launch
+	 * from lti_tools - the dependency means that this will not find content items that do
+	 * not have an associated tool.
+	 */
 	public List<Map<String, Object>> getContentsDao(String search, String order, int first,
 			int last, String siteId, boolean isAdminRole) {
 
+		// It is important that any tables/columns added for the purposes of display or searching be
+		// LEFT JOIN - *any* INNER JOIN will function as a WHERE clause and will hide content
         String concatSearch = ("mysql".equals(m_sql.getVendor())) ?  "CONCAT_WS('', lti_content.launch, lti_tools.launch)" : "(lti_content.launch || lti_tools.launch)";
         String extraSelect = "SAKAI_SITE.TITLE AS SITE_TITLE, ssp1.VALUE AS SITE_CONTACT_NAME, ssp2.VALUE AS SITE_CONTACT_EMAIL, lti_tools.launch as URL, "+concatSearch+" AS searchURL";
-        String joinClause = "JOIN SAKAI_SITE ON lti_content.SITE_ID = SAKAI_SITE.SITE_ID"
+        String joinClause = "LEFT JOIN SAKAI_SITE ON lti_content.SITE_ID = SAKAI_SITE.SITE_ID"
         		+ " LEFT JOIN SAKAI_SITE_PROPERTY ssp1 ON (lti_content.SITE_ID = ssp1.SITE_ID AND ssp1.name = 'contact-name')"
         		+ " LEFT JOIN SAKAI_SITE_PROPERTY ssp2 ON (lti_content.SITE_ID = ssp2.SITE_ID AND ssp2.name = 'contact-email')"
-        		+ " JOIN lti_tools ON (lti_content.tool_id = lti_tools.id)";
-        
+			+ " LEFT JOIN lti_tools ON (lti_content.tool_id = lti_tools.id)";
+
         String propertyKey = serverConfigurationService.getString(LTI_SITE_ATTRIBUTION_PROPERTY_KEY, LTI_SITE_ATTRIBUTION_PROPERTY_KEY_DEFAULT);
         if (StringUtils.isNotEmpty(propertyKey)) {
             extraSelect += ", ssp3.VALUE as ATTRIBUTION";
             joinClause = joinClause + " LEFT JOIN SAKAI_SITE_PROPERTY ssp3 ON (lti_content.SITE_ID = ssp3.SITE_ID AND ssp3.name = '" + propertyKey + "')";
         }
-        
+
         final String[] fields = (String[])ArrayUtils.addAll(LTIService.CONTENT_MODEL, LTIService.CONTENT_EXTRA_FIELDS);
         if (order != null) {
             order = foorm.orderCheck(order, "lti_content", fields);
             if (order == null) {
                 throw new IllegalArgumentException("order must be [table.]field [asc|desc]");
             }
         }
+	// TODO: SAK-32704 - Resolve the different ways to do search
         search = foorm.searchCheck(search, "lti_content", fields);
-        
+
         List<Map<String, Object>> contents = getThingsDao("lti_content", LTIService.CONTENT_MODEL, extraSelect, joinClause, search, null, order, first, last, siteId, isAdminRole);
         for (Map<String, Object> content : contents) {
             content.put("launch_url", getContentLaunch(content));
@@ -448,17 +456,20 @@ public List<Map<String, Object>> getContentsDao(String search, String order, int
 	}
 
 	/**
-	 * 
+	 *
 	 * {@inheritDoc}
 	 * 
 	 * @see org.sakaiproject.lti.api.LTIService#countContentsDao(java.lang.String,
 	 *      java.lang.String, boolean)
 	 */
 	public int countContentsDao(String search, String siteId, boolean isAdminRole) {
-        String joinClause = "JOIN SAKAI_SITE ON lti_content.SITE_ID = SAKAI_SITE.SITE_ID"
+		// It is important that any tables/columns added for the purposes of display or searching be
+		// LEFT JOIN - *any* INNER JOIN will function as a WHERE clause and will hide content
+		// items from the admin UI - so they will not be seen and cannot be repaired
+        String joinClause = "LEFT JOIN SAKAI_SITE ON lti_content.SITE_ID = SAKAI_SITE.SITE_ID"
         		+ " LEFT JOIN SAKAI_SITE_PROPERTY ssp1 ON (lti_content.SITE_ID = ssp1.SITE_ID AND ssp1.name = 'contact-name')"
         		+ " LEFT JOIN SAKAI_SITE_PROPERTY ssp2 ON (lti_content.SITE_ID = ssp2.SITE_ID AND ssp2.name = 'contact-email')"
-        		+ " JOIN lti_tools ON (lti_content.tool_id = lti_tools.id)";
+			+ " LEFT JOIN lti_tools ON (lti_content.tool_id = lti_tools.id)";
         final String propertyKey = serverConfigurationService.getString(LTI_SITE_ATTRIBUTION_PROPERTY_KEY, LTI_SITE_ATTRIBUTION_PROPERTY_KEY_DEFAULT);
         if (StringUtils.isNotEmpty(propertyKey)) {
             joinClause = joinClause + " LEFT JOIN SAKAI_SITE_PROPERTY ssp3 ON (lti_content.SITE_ID = ssp3.SITE_ID AND ssp3.name = '" + propertyKey + "')";

basiclti/basiclti-tool/src/bundle/ltitool.properties

@@ -161,6 +161,8 @@ deploy.activate.notools=There are no tools to activate
 deploy.parse.exception=Unable to parse provider profile
 deploy.delete=Deleting Deployment
 deploy.delete.sure=Are you sure you want to delete this deployment?
+deploy.delete.tool=Delete: 
+deploy.delete.tools=The tools below were installed by this deployment. If you delete these tools, all of the links to these tools in all the sites in this system will also be deleted.  You can delete the deployment without deleting the tools.
 deploy.data=This deployment supports :tools tools and :contents placements in :sites sites.
 add.deploy=Install LTI 2.x Tool
 deploy.description.system=Deployments currently in this system

basiclti/basiclti-tool/src/java/org/sakaiproject/blti/tool/LTIAdminTool.java

@@ -746,42 +746,19 @@ public void doToolDelete(RunData data, Context context)
 			return;
 		}
 		Long key = new Long(id);
-		if ( ltiService.deleteTool(key, getSiteId(state)) )
-		{
-			state.setAttribute(STATE_SUCCESS,rb.getString("success.deleted"));
-
-			// remove all content object and site links if any
-			// this is for the "site tools" panel
-			List<Map<String,Object>> contents = ltiService.getContents(null,null,0,5000, getSiteId(state));
-			for ( Map<String,Object> content : contents ) {
-
-				Long tool_id_long = null;
-				try{
-					tool_id_long = new Long(content.get(LTIService.LTI_TOOL_ID).toString());
-					if (tool_id_long.equals(key))
-					{
-						// the content with same tool id
-						// remove the content link first
-						String content_id = content.get(LTIService.LTI_ID).toString();
-						Long content_key = content_id == null ? null:new Long(content_id);
-
-						//TODO: how to handle the errors in content link and content deletion?
-						// remove the external tool content site link
-						ltiService.deleteContentLink(content_key, getSiteId(state));
-						// remove the external tool content
-						ltiService.deleteContent(content_key, getSiteId(state));
-					}
-				}
-				catch (Exception e)
-				{
-					// log the error
-					M_log.error("error parsing tool id " + content.get(LTIService.LTI_TOOL_ID));
-				}
-			}
-
+
+		// Delete the tool and all associated content items and site links
+		List<String> errors = ltiService.deleteToolAndContents(key, getSiteId(state));
+		String errorNote = "";
+		for(String errstr: errors ) {
+			M_log.error(errstr);
+			errorNote += "<br/>"+errstr;
+		}
+
+		if ( errors.size() < 1 ) {
 			switchPanel(state, "ToolSystem");
 		} else {
-			addAlert(state,rb.getString("error.delete.fail"));
+			addAlert(state,rb.getString("error.delete.fail")+errorNote);
 			switchPanel(state, "ToolSystem");
 		}
 	}
@@ -1495,6 +1472,10 @@ public String buildDeployDeletePanelContext(VelocityPortlet portlet, Context con
 			return "lti_deploy_system";
 		}
 
+		// Retrieve all the tools that are related to this deploy
+		List<Map<String,Object>> tools = ltiService.getTools("deployment_id = "+key,null,0,0, getSiteId(state));
+		context.put("tools", tools);
+
 		Map<String,Object> deploy  = deploys.get(0);
 		context.put("deploy",deploy);
 
@@ -1531,6 +1512,29 @@ public void doDeployDelete(RunData data, Context context)
 			return;
 		}
 		Long key = new Long(id);
+
+		// Retrieve all the tools that are related to this deploy
+		List<Map<String,Object>> tools = ltiService.getTools("deployment_id = "+key,null,0,0, getSiteId(state));
+		String errmsg = "";
+		for ( Map<String,Object> tool : tools ) {
+			Long tool_id = foorm.getLongNull(tool.get("id"));
+			String parm = "delete_"+tool_id;
+			String val = data.getParameters().getString(parm);
+			if ( val == null || ! "on".equals(val) ) continue;
+			// Delete the tool and all associated content items and site links
+			List<String> errors = ltiService.deleteToolAndContents(tool_id, getSiteId(state));
+			for(String errstr: errors ) {
+				M_log.error(errstr);
+				errmsg += "<br/>"+errstr;
+			}
+		}
+
+		if ( errmsg.length() > 0 ) {
+			addAlert(state,rb.getString("error.delete.fail")+errmsg);
+			switchPanel(state, "DeploySystem");
+			return;
+		}
+
 		// also remove the link
 		if ( ltiService.deleteDeployDao(key) )
 		{

basiclti/basiclti-tool/src/webapp/vm/lti_deploy_delete.vm

@@ -7,7 +7,24 @@
 	<div class="alertMessage">$tlang.getString("deploy.delete.sure")</div><div style="display:block;clear:both" ></div>
 	<form action="#toolForm("")" method="post" name="customizeForm" >
 		$formOutput
+		<p class="row">
 		$deployData
+		</p>
+#if ($tools.size() > 0)
+	<p class="row">
+	<div class="alertMessage">
+	$tlang.getString("deploy.delete.tools")
+	</div>
+	</p>
+       <ul style="list-style-type: none;">
+                #foreach($tool in $tools)
+                <li>
+			<input type="checkbox" id="delete_${tool.get("id")}" name="delete_${tool.get("id")}">
+			<label for="delete_${tool.get("id")}">$tlang.getString("deploy.delete.tool") $validator.escapeHtml($tool.get("title"))</label>
+                </li>
+                #end
+        </ul>
+#end
                 <input type="hidden" name="sakai_csrf_token" value="$sakai_csrf_token" />
                 <input type="hidden" name="id" value="$deploy.get("id")" />
 		<p class="act">