Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
SAK-50789 Lessons improve import and feedback for LTI tools
Browse files Browse the repository at this point in the history
csev committed Jan 28, 2025
1 parent 5efb25a commit 2f6a233
Showing 8 changed files with 109 additions and 47 deletions.
7 changes: 5 additions & 2 deletions lessonbuilder/api/src/resources/lessons.properties
Original file line number Diff line number Diff line change
@@ -675,8 +675,7 @@ simplepage.format.heading=Display Options
simplepage.format.window=Open in New Window
simplepage.format.inline=Embed on page
simplepage.format.page=Open in New Lessons tool Page with 'next' and 'back' buttons
simplepage.format.item_removed=<p style='font-family:arial, helvetica; padding:4px; font-size:80%; border:2px solid black'><span style='color:#c00'>ERROR:</span> The item that should have been displayed here has been removed. Please use the "Edit" button next to this item and pick "Change External Tool" to recreate the item or choose a new one.</p>

simplepage.format.item_removed_text=The item that should have been displayed here has been removed. Please use the "Edit" button next to this item and pick "Change External Tool" to recreate the item or choose a new one.
simplepage.more-tools=More Tools
simplepage.forum-descrip=Link to a Forum or Topic
simplepage.blti-descrip=Link to a web service that uses the IMS LTI standard to integrate with this system
@@ -961,3 +960,7 @@ simplepage.max-file-upload-size=The upload size limit of
simplepage.max-file-upload-size-save=MB has been exceeded. Remove one or more files below to proceed. Other files may be uploaded assuming the total file size does not exceed this limit.

lessons_comment=Lessons Comment

lti.tool.missing=Content Item has no LTI tool, please re-select
lti.tool.import.incomplete=LTI tool for this item needs further configuration
lti.tool.is.draft=LTI tool for this item is still draft, needs configuration
Original file line number Diff line number Diff line change
@@ -33,6 +33,8 @@
import java.util.HashMap;
import java.util.Properties;

import org.apache.commons.lang3.StringUtils;

import org.json.simple.JSONObject;

import lombok.extern.slf4j.Slf4j;
@@ -51,6 +53,7 @@
import org.sakaiproject.site.api.ToolConfiguration;
import org.sakaiproject.site.api.SiteService;
import org.sakaiproject.tool.api.ToolManager;
import org.sakaiproject.util.ResourceLoader;

import org.tsugi.lti.LTIUtil;
import org.sakaiproject.lti.util.SakaiLTIUtil;
@@ -67,6 +70,7 @@
public class BltiEntity implements LessonEntity, BltiInterface {
private static Cache bltiCache = null;
protected static final int DEFAULT_EXPIRATION = 10 * 60;
protected static ResourceLoader rb = new ResourceLoader("lessons");

private SimplePageBean simplePageBean;

@@ -313,27 +317,37 @@ public String getIcon() {
}

private String getErrorUrl() {
return "javascript:document.write('" + messageLocator.getMessage("simplepage.format.item_removed").replace("'", "\\'") + "')";
return "javascript:alert('" + messageLocator.getMessage("simplepage.format.item_removed_text").replace("'", "\\'") + "')";
}

public String getEditNote() {
loadContent();
if ( content == null ) return null; // Lessons will show *deleted*

if ( tool == null ) {
return rb.getString("lti.tool.missing");
}


String siteId = getSiteId();
if ( StringUtils.isNotEmpty(siteId) && siteId.equals((String) tool.get(LTIService.LTI_SITE_ID))
&& LTIService.LTI_SECRET_INCOMPLETE.equals((String) tool.get(LTIService.LTI_SECRET))
&& LTIService.LTI_SECRET_INCOMPLETE.equals((String) tool.get(LTIService.LTI_CONSUMERKEY)) ) {
return rb.getString("lti.tool.import.incomplete");
}

if ( ltiService.isDraft(tool) ) {
return rb.getString("lti.tool.is.draft");
}

return null;
}

// TODO: Concern regarding the lack of the returnUrl when this is called
public String getUrl() {
loadContent();
// If I return null here, it appears that I cause an NPE in LB
if ( content == null ) return getErrorUrl();
String ret = (String) content.get("launch_url");
if ( ltiService != null && tool != null && ltiService.isMaintain(getSiteId())
&& LTIService.LTI_SECRET_INCOMPLETE.equals((String) tool.get(LTIService.LTI_SECRET))
&& LTIService.LTI_SECRET_INCOMPLETE.equals((String) tool.get(LTIService.LTI_CONSUMERKEY)) ) {

String toolId = getCurrentTool("sakai.siteinfo");
if ( toolId != null ) {
ret = editItemUrl(toolId);
ret = ret + "&secretonly=true";
return ret;
}
}

if ( ret == null ) return getErrorUrl();
ret = ServerConfigurationService.getServerUrl() + ret;
return ret;
}
@@ -457,6 +471,7 @@ public String doImportTool(String launchUrl, String bltiTitle, String strXml, St
// Allow the content item to choose open in popup so the user can change it in the Lessons UI
props.setProperty(LTIService.LTI_NEWPAGE, "2");
props.setProperty(LTIService.LTI_XMLIMPORT,strXml);
props.setProperty(LTIService.LTI13,"0");
if (custom != null)
props.setProperty(LTIService.LTI_CUSTOM, custom);
Object result = ltiService.insertTool(props, simplePageBean.getCurrentSiteId());
Original file line number Diff line number Diff line change
@@ -112,6 +112,10 @@ public interface LessonEntity {
public String getTitle();
public String getDescription();
public String getUrl();
// Returns a note to display near the link for editors (i.e. like Deleted)
default public String getEditNote() {
return null;
}
public Date getDueDate();
// for forums, where we have a hiearchy of topics
public int getLevel();
Original file line number Diff line number Diff line change
@@ -1530,6 +1530,7 @@ public void printSubpage(List<SimplePageItem> itemList, boolean first, UIBranchC
// javascript that prepares
// the jQuery dialogs
String itemGroupString = null;
String editNote = null;
boolean entityDeleted = false;
boolean notPublished = false;
if (canEditPage) {
@@ -1606,19 +1607,18 @@ else if (assignment.notPublished())
UIOutput.make(tableRow, "type", "b");
LessonEntity blti = (bltiEntity == null ? null : bltiEntity.getEntity(i.getSakaiId()));
if (blti != null) {
String editUrl = blti.editItemUrl(simplePageBean);
if (editUrl != null)
UIOutput.make(tableRow, "edit-url", editUrl);
UIOutput.make(tableRow, "item-format", i.getFormat());

if (i.getHeight() != null)
UIOutput.make(tableRow, "item-height", i.getHeight());
itemGroupString = simplePageBean.getItemGroupString(i, null, true);
UIOutput.make(tableRow, "item-groups", itemGroupString );
if (!blti.objectExists())
entityDeleted = true;
else if (blti.notPublished())
notPublished = true;
String editUrl = blti.editItemUrl(simplePageBean);
editNote = blti.getEditNote();
if (editUrl != null) UIOutput.make(tableRow, "edit-url", editUrl);
UIOutput.make(tableRow, "item-format", i.getFormat());

if (i.getHeight() != null) UIOutput.make(tableRow, "item-height", i.getHeight());
itemGroupString = simplePageBean.getItemGroupString(i, null, true);
UIOutput.make(tableRow, "item-groups", itemGroupString );
if (!blti.objectExists())
entityDeleted = true;
else if (blti.notPublished())
notPublished = true;
}
} else if (i.getType() == SimplePageItem.FORUM) {
UIOutput.make(tableRow, "extra-info");
@@ -1656,6 +1656,7 @@ else if (forum.notPublished())

} // end of canEditPage


if (i.getType() == SimplePageItem.PAGE) {
UIOutput.make(tableRow, "type", "page");
UIOutput.make(tableRow, "page-next", Boolean.toString(i.getNextPage()));
@@ -1708,8 +1709,9 @@ else if (lessonEntity.notPublished())
notPublished = true;
break;
case SimplePageItem.BLTI:
if (bltiEntity != null)
lessonEntity = bltiEntity.getEntity(i.getSakaiId());
if (bltiEntity != null) {
lessonEntity = bltiEntity.getEntity(i.getSakaiId());
}
if (lessonEntity != null)
itemGroupString = simplePageBean.getItemGroupString(i, null, true);
if (!lessonEntity.objectExists())
@@ -1749,6 +1751,10 @@ else if (lessonEntity.notPublished())
else
itemGroupString = messageLocator.getMessage("simplepage.not-published");
}
if ( StringUtils.isNotEmpty(editNote) ) {
if (StringUtils.isEmpty(itemGroupString) ) itemGroupString= "";
itemGroupString = itemGroupString + " " + editNote;
}
if (entityDeleted) {
if (itemGroupString != null)
itemGroupString = itemGroupString + " " +
3 changes: 3 additions & 0 deletions lti/lti-api/src/java/org/sakaiproject/lti/api/LTIService.java
Original file line number Diff line number Diff line change
@@ -348,6 +348,9 @@ public interface LTIService extends LTISubstitutionsFilter {

String validateTool(Map<String, Object> newProps);

// Returns whether or not a tool needs further configuration
boolean isDraft(Map<String, Object> tool);

Object insertTool(Properties newProps, String siteId);

Object insertTool(Map<String, Object> newProps, String siteId);
33 changes: 21 additions & 12 deletions lti/lti-common/src/java/org/sakaiproject/lti/util/SakaiLTIUtil.java
Original file line number Diff line number Diff line change
@@ -1178,7 +1178,7 @@ public static String[] postLaunchHTML(Map<String, Object> content, Map<String, O
return postError("<p>" + getRB(rb, "error.tool.partial", "Tool item is incomplete, missing a key and secret.") + "</p>");
}

boolean isLTI13 = isLTI13(tool, content);
boolean isLTI13 = isLTI13(tool);

log.debug("isLTI13={}", isLTI13);

@@ -1488,7 +1488,7 @@ public static String[] postContentItemSelectionRequest(Long toolKey, Map<String,
// If secret is encrypted, decrypt it
secret = decryptSecret(secret);

boolean isLTI13 = isLTI13(tool, null);
boolean isLTI13 = isLTI13(tool);

log.debug("isLTI13={}", isLTI13);

@@ -3472,7 +3472,7 @@ public static Map<String, String> deserializeMap(String mapSer) {
}

/**
* Converts a string from a semicolon-separated list of legacy lti roles mapped ot a modern LTI role to a
* Converts a string from a semicolon-separated list of legacy lti roles mapped to a modern LTI role to a
* Map<String, String>. Each role mapping should be of the form:
* Learner=http://purl.imsglobal.org/vocab/lis/v2/membership#Learner
*/
@@ -3481,19 +3481,28 @@ public static Map<String, String> convertLegacyRoleMapPropToMap(String roleMapPr
}

/**
* Check if we are an LTI 1.3 launch or not
* Check if we are an LTI 1.1 launch or not
*/
public static boolean isLTI13(Map<String, Object> tool, Map<String, Object> content) {
// 0=inherit from tool, 1=LTI 1.1, 2=LTI 1.3
if ( content != null ) {
Long contentLTI13 = Foorm.getLong(content.get(LTIService.LTI13));
if ( contentLTI13.equals(2L)) return true;
if ( contentLTI13.equals(1L)) return false;
}
public static boolean isLTI11(Map<String, Object> tool) {
if ( tool == null ) return false;
Long toolLTI13 = Foorm.getLong(tool.get(LTIService.LTI13));
if ( toolLTI13.equals(LTIService.LTI13_LTI11) ) return true;
if ( toolLTI13.equals(LTIService.LTI13_LTI13) ) return false;
if ( toolLTI13.equals(LTIService.LTI13_BOTH) ) return true;
return true;
}


/**
* Check if we are an LTI 1.3 launch or not
*/
public static boolean isLTI13(Map<String, Object> tool) {
if ( tool == null ) return false;
Long toolLTI13 = Foorm.getLong(tool.get(LTIService.LTI13));
return ! toolLTI13.equals(0L);
if ( toolLTI13.equals(LTIService.LTI13_LTI11) ) return false;
if ( toolLTI13.equals(LTIService.LTI13_LTI13) ) return true;
if ( toolLTI13.equals(LTIService.LTI13_BOTH) ) return true;
return false; // Default of null or other funky value is LTI 1.1
}

/**
Original file line number Diff line number Diff line change
@@ -479,6 +479,28 @@ public String validateTool(Properties newProps) {
return validateTool((Map) newProps);
}

@Override
public boolean isDraft(Map<String, Object> tool) {
boolean retval = true;
if ( tool == null ) return retval;
if ( StringUtils.isEmpty((String) tool.get(LTI_LAUNCH)) ) return true;
if ( SakaiLTIUtil.isLTI11(tool) ) {
String consumerKey = (String) tool.get(LTI_CONSUMERKEY);
String consumerSecret = (String) tool.get(LTI_SECRET);
if ( StringUtils.isNotEmpty(consumerSecret) && StringUtils.isNotEmpty(consumerSecret)
&& (! LTI_SECRET_INCOMPLETE.equals(consumerSecret))
&& (! LTI_SECRET_INCOMPLETE.equals(consumerKey)) ) retval = false;
}

if ( SakaiLTIUtil.isLTI13(tool)
&& StringUtils.isNotEmpty((String) tool.get(LTI13_CLIENT_ID))
&& StringUtils.isNotEmpty((String) tool.get(LTI13_TOOL_KEYSET))
&& StringUtils.isNotEmpty((String) tool.get(LTI13_TOOL_ENDPOINT))
&& StringUtils.isNotEmpty((String) tool.get(LTI13_TOOL_REDIRECT))) retval = false;

return retval;
}

@Override
public String validateTool(Map<String, Object> newProps) {
StringBuffer sb = new StringBuffer();
Original file line number Diff line number Diff line change
@@ -345,7 +345,7 @@ private boolean sanityCheck(HttpServletRequest req, HttpServletResponse res,
{

String oidc_endpoint = (String) tool.get(LTIService.LTI13_TOOL_ENDPOINT);
if (SakaiLTIUtil.isLTI13(tool, content) && StringUtils.isBlank(oidc_endpoint) ) {
if (SakaiLTIUtil.isLTI13(tool) && StringUtils.isBlank(oidc_endpoint) ) {
String errorMessage = "<p>" + SakaiLTIUtil.getRB(rb, "error.no.oidc_endpoint", "Missing oidc_endpoint value for LTI 1.3 launch") + "</p>";
org.tsugi.lti.LTIUtil.sendHTMLPage(res, errorMessage);
return false;
@@ -414,7 +414,7 @@ public void handleAccess(HttpServletRequest req, HttpServletResponse res, Refere
// Sanity check for missing config data
if ( ! sanityCheck(req, res, null, tool, rb) ) return;

if (SakaiLTIUtil.isLTI13(tool, null) && StringUtils.isNotBlank(oidc_endpoint) &&
if (SakaiLTIUtil.isLTI13(tool) && StringUtils.isNotBlank(oidc_endpoint) &&
( StringUtils.isEmpty(state) || StringUtils.isEmpty(state) ) ) {
redirectOIDC(req, res, null, tool, oidc_endpoint, rb);
return;
@@ -494,7 +494,7 @@ else if ( refId.startsWith("content:") && refId.length() > 8 )
// Sanity check for missing config data
if ( ! sanityCheck(req, res, content, tool, rb) ) return;

if (SakaiLTIUtil.isLTI13(tool, content) && StringUtils.isNotBlank(oidc_endpoint) &&
if (SakaiLTIUtil.isLTI13(tool) && StringUtils.isNotBlank(oidc_endpoint) &&
(StringUtils.isEmpty(state) || StringUtils.isEmpty(nonce) ) ) {
redirectOIDC(req, res, content, tool, oidc_endpoint, rb);
return;

0 comments on commit 2f6a233

Please sign in to comment.