Index: site-manage-tool/tool/src/bundle/sitesetupgeneric.properties =================================================================== --- site-manage-tool/tool/src/bundle/sitesetupgeneric.properties (revision 310642) +++ site-manage-tool/tool/src/bundle/sitesetupgeneric.properties (working copy) @@ -1199,6 +1199,8 @@ ediacc.notifyEnabledLocalDisabledGlobal = This option has been disabled system-wide. You will not receive email notifications when someone joins your site. ediacc.excludeEnabledLocalDisabledGlobal = This option has been disabled system-wide. This site will appear in the publicly available list of joinable sites. ediacc.limitEnabledLocalDisabledGlobal = This option has been disabled system-wide. Any user can join this site. +ediacc.selectAtLeastOneAccountType = You must select at least one account type below. +ediacc.noAccountTypesSelected = You chose to limit the joining of this site to users with specific accounts, but no account types were selected. Either select at least one account type that can join, or deselect this account type limitation. # SAK-22384 MathJax support sinfo.mathjax.name=LaTeX Index: site-manage-tool/tool/src/java/org/sakaiproject/site/tool/InvalidJoinableSiteSettingsException.java =================================================================== --- site-manage-tool/tool/src/java/org/sakaiproject/site/tool/InvalidJoinableSiteSettingsException.java (revision 0) +++ site-manage-tool/tool/src/java/org/sakaiproject/site/tool/InvalidJoinableSiteSettingsException.java (working copy) @@ -0,0 +1,61 @@ +package org.sakaiproject.site.tool; + +import org.sakaiproject.util.ResourceLoader; + +// Introduced for SAK-26626. +/** + * Thrown when joinable site settings are invalid. + * Stores resource message keys/params within the exception, this way user facing messages can be decided at the time of the exception. + * Feel free to reuse this pattern elsewhere in Sakai ;) + * @author bbailla2 + */ +public class InvalidJoinableSiteSettingsException extends RuntimeException +{ + protected String messageKey = ""; + protected Object[] messageArgs = null; + + /** + * @param message the Exception message + * @param messageKey the Resource Loader key to be used for user facing messages + */ + public InvalidJoinableSiteSettingsException(String message, String messageKey) + { + super(message); + this.messageKey = messageKey; + } + + /** + * @param message the Exception message + * @param messageKey the Resource Loader key to be used for user facing messages + * @param messageArgs the arguments to be passed to the Resource Loader to populate user facing messages + */ + public InvalidJoinableSiteSettingsException(String message, String messageKey, Object[] messageArgs) + { + super(message); + this.messageKey = messageKey; + this.messageArgs = messageArgs; + } + + public String getMessageKey() + { + return messageKey; + } + + public Object[] getMessageArgs() + { + return messageArgs; + } + + /** + * @param rb the ResourceLoader to fetch a formatted message from + * @return the formatted message from the specified ResourceLoader using this object's messageKey and messageArgs + */ + public String getFormattedMessage(ResourceLoader rb) + { + if (messageArgs == null) + { + return rb.getString(messageKey); + } + return rb.getFormattedMessage(messageKey, messageArgs); + } +} Index: site-manage-tool/tool/src/java/org/sakaiproject/site/tool/JoinableSiteSettings.java =================================================================== --- site-manage-tool/tool/src/java/org/sakaiproject/site/tool/JoinableSiteSettings.java (revision 310642) +++ site-manage-tool/tool/src/java/org/sakaiproject/site/tool/JoinableSiteSettings.java (working copy) @@ -648,9 +648,14 @@ * @param state * the object to get the settings from * @return status (true/false) + * @throws InvalidJoinableSiteSettingsException when the settings in the state are invalid */ public static boolean updateSitePropertiesFromStateOnSiteInfoSaveGlobalAccess( ResourcePropertiesEdit props, SessionState state ) { + // SAK-26626 --bbailla2 + // validate the state, throw an InvalidJoinableSiteSettingsException if the state's settings are invalid + validateJoinableSiteSettings(state); + if( props == null || state == null ) { return false; @@ -683,9 +688,14 @@ * @param state * the object to get the settings from * @return status (true/false) + * @throws InvalidJoinableSiteSettingsException when the settings in the state are invalid */ public static boolean updateSitePropertiesFromStateOnSiteUpdate( ResourcePropertiesEdit props, SessionState state ) { + // SAK-26626 --bbailla2 + // validate the state, throw an InvalidJoinableSiteSettingsException if the state's settings are invalid + validateJoinableSiteSettings(state); + if( props == null || state == null ) { return false; @@ -1269,6 +1279,38 @@ propertyList = sb.toString(); props.addProperty( SITE_PROP_JOIN_SITE_ACCOUNT_TYPES, propertyList ); } + + // SAK-26626 --bbailla2 + /** + * Validates the joinable site settings in the state + * @throws InvalidJoinableSiteSettingsException when the settings in the state are invalid + */ + public static void validateJoinableSiteSettings(SessionState state) + { + String attribute = ""; + if (siteService.isGlobalJoinLimitByAccountTypeEnabled()) + { + attribute = state.getAttribute(STATE_JOIN_SITE_LIMIT_BY_ACCOUNT_TYPE).toString(); + if (TRUE_STRING.equalsIgnoreCase(attribute)) + { + boolean accountTypeSelected = false; + for (String account : siteService.getAllowedJoinableAccountTypes()) + { + attribute = state.getAttribute(STATE_JOIN_SITE_ACCOUNT_TYPE_PREFIX + account).toString(); + if (TRUE_STRING.equalsIgnoreCase(attribute)) + { + accountTypeSelected = true; + break; + } + } + + if (!accountTypeSelected) + { + throw new InvalidJoinableSiteSettingsException("Limit join to specific accounts selected, but no accounts specified", "ediacc.noAccountTypesSelected"); + } + } + } + } /** * Read in form field values from the ParameterParser and update/remove values from the state Index: site-manage-tool/tool/src/java/org/sakaiproject/site/tool/SiteAction.java =================================================================== --- site-manage-tool/tool/src/java/org/sakaiproject/site/tool/SiteAction.java (revision 310642) +++ site-manage-tool/tool/src/java/org/sakaiproject/site/tool/SiteAction.java (working copy) @@ -8601,8 +8601,16 @@ addAlert(state, rb.getString("java.joinsite") + " "); } - // bjones86 - SAK-24423 - update site properties for joinable site settings - JoinableSiteSettings.updateSitePropertiesFromStateOnSiteUpdate( sEdit.getPropertiesEdit(), state ); + // SAK-26626 - handle invalid settings --bbailla2 + try + { + // bjones86 - SAK-24423 - update site properties for joinable site settings + JoinableSiteSettings.updateSitePropertiesFromStateOnSiteUpdate( sEdit.getPropertiesEdit(), state ); + } + catch (InvalidJoinableSiteSettingsException invalidSettingsException) + { + addAlert(state, invalidSettingsException.getFormattedMessage(rb)); + } } else if ( !joinable || (!joinable && ServerConfigurationService.getBoolean(CONVERT_NULL_JOINABLE_TO_UNJOINABLE, true))) { @@ -9876,8 +9884,16 @@ String joinerRole = (String) state.getAttribute("form_joinerRole"); s.setJoinerRole(joinerRole); - // bjones86 - SAK-24423 - update site properties for joinable site settings - JoinableSiteSettings.updateSitePropertiesFromStateOnSiteInfoSaveGlobalAccess( s.getPropertiesEdit(), state ); + // SAK-26626 - handle invalid settings --bbailla2 + try + { + // bjones86 - SAK-24423 - update site properties for joinable site settings + JoinableSiteSettings.updateSitePropertiesFromStateOnSiteInfoSaveGlobalAccess( s.getPropertiesEdit(), state ); + } + catch (InvalidJoinableSiteSettingsException invalidSettingsException) + { + addAlert(state, invalidSettingsException.getFormattedMessage(rb)); + } } if (state.getAttribute(STATE_MESSAGE) == null) { Index: site-manage-tool/tool/src/webapp/css/site-manage.css =================================================================== --- site-manage-tool/tool/src/webapp/css/site-manage.css (revision 310642) +++ site-manage-tool/tool/src/webapp/css/site-manage.css (working copy) @@ -47,6 +47,10 @@ min-width: 30em; } +#joinLimitByAccountType +{ + margin-bottom: 1em; +} #templateSettings, #siteTypeList, #archiveSettings{ padding:.5em; background:#eee; Index: site-manage-tool/tool/src/webapp/vm/sitesetup/chef_site-siteInfo-editAccess.vm =================================================================== --- site-manage-tool/tool/src/webapp/vm/sitesetup/chef_site-siteInfo-editAccess.vm (revision 310642) +++ site-manage-tool/tool/src/webapp/vm/sitesetup/chef_site-siteInfo-editAccess.vm (working copy) @@ -60,7 +60,86 @@ } } - + ## SAK-26626 --bbailla2 + ## A mechanism to validate this page; it will enable or disable the continue/update button + function validatePage() + { + var valid = true; + + #if ($!site) + #set($submitButtonId="updateButton") + #else + #set($submitButtonId="continueButton") + #end + var submitButton = document.getElementById("$submitButtonId"); + + if (!limitByAccountTypesValidation()) + { + valid = false; + } + + if (valid) + { + submitButton.removeAttribute("disabled"); + } + else + { + submitButton.setAttribute("disabled", "disabled"); + } + + ## limitByAccountTypesValidation can cause an info message to display. Avoid double scroll bars: + utils.resizeFrame(); + } + + ## SAK-26626 --bbailla2 + ## Returns true iff the limitByAccountType checkboxes are in a valid state. + ## Also responsible for the visibility of the "You must select at least one account type below" message + function limitByAccountTypesValidation() + { + ## assume the checkboxes are in a valid state, and that we don't need to alert the user to select an account + var valid = true; + var displayJoinLimitInfo = false; + + ## the 'Limit join to specific accounts' checkbox + var chkJoinLimitByAccountType = document.getElementById("chkJoinLimitByAccountType"); + if (chkJoinLimitByAccountType && chkJoinLimitByAccountType.checked) + { + ## get the account type checkboxes + var joinAccountTypesDiv = document.getElementById("joinerAccountTypes"); + var chkAccountTypes = joinAccountTypesDiv.getElementsByTagName('input'); + + ## determine if at least one is checked + var atLeastOneChecked = false; + for (var i = 0; i < chkAccountTypes.length; i++) + { + if (chkAccountTypes[i].checked) + { + atLeastOneChecked = true; + break; + } + } + + if (!atLeastOneChecked) + { + ## 'Limit join to specific accounts' is checked, but no accounts are checked; the page is invalid + displayJoinLimitInfo = true; + valid = false; + } + } + + ## Control the visibility of the "You must select at least one account type below" message + var joinLimitInfoDiv = document.getElementById("joinLimitInfoDiv"); + if (displayJoinLimitInfo) + { + joinLimitInfoDiv.removeAttribute("style"); + } + else + { + joinLimitInfoDiv.setAttribute("style", "display:none;"); + } + + return valid; + } // -->
+
+ #if( $joinLimitEnabledLocalDisabledGlobal ) @@ -300,7 +382,7 @@ #if( !$category ) #foreach( $accType in $joinableAccountTypes )
- - #else - + #end