Skip to content

Complete migration from srcURL and returnURL to returnUrl #6407

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
labkey-jeckels merged 2 commits into from
Mar 5, 2025
Merged

Complete migration from srcURL and returnURL to returnUrl #6407

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8b6b359
Complete migration from srcURL and returnURL to returnUrl
labkey-jeckels Mar 4, 2025
71b4cb6
Code review suggestions
labkey-jeckels Mar 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 33 additions & 33 deletions announcements/src/org/labkey/announcements/AnnouncementsController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,11 +303,11 @@ public void addNavTrail(NavTree root)
}
}

public static ActionURL getAdminEmailURL(Container c, @Nullable URLHelper returnURL)
public static ActionURL getAdminEmailURL(Container c, @Nullable URLHelper returnUrl)
{
ActionURL url = urlProvider(AdminUrls.class).getNotificationsURL(c);
if (returnURL != null)
url.addReturnURL(returnURL);
if (returnUrl != null)
url.addReturnUrl(returnUrl);

return url;
}
Expand Down Expand Up @@ -381,7 +381,7 @@ public ModelAndView getConfirmView(AnnouncementDeleteForm form, BindException er
@Override
public @NotNull URLHelper getSuccessURL(AnnouncementDeleteForm form)
{
return form.getReturnURLHelper(new ActionURL(BeginAction.class, getContainer()));
return form.getReturnUrlHelper(new ActionURL(BeginAction.class, getContainer()));
}

@Override
Expand Down Expand Up @@ -461,7 +461,7 @@ public static ActionURL getDeleteResponseURL(Container c, String entityId, URLHe
{
ActionURL url = new ActionURL(DeleteResponseAction.class, c);
url.addParameter("entityId", entityId);
url.addReturnURL(returnUrl);
url.addReturnUrl(returnUrl);

return url;
}
Expand Down Expand Up @@ -693,7 +693,7 @@ public static ActionURL getAdminURL(Container c, @Nullable URLHelper returnUrl)
{
ActionURL url = new ActionURL(CustomizeAction.class, c);
if (returnUrl != null)
url.addReturnURL(returnUrl);
url.addReturnUrl(returnUrl);

return url;
}
Expand All @@ -705,7 +705,7 @@ public class CustomizeAction extends FormViewAction<Settings>
@Override
public URLHelper getSuccessURL(Settings form)
{
return form.getReturnURLHelper();
return form.getReturnUrlHelper();
}

@Override
Expand All @@ -714,7 +714,7 @@ public ModelAndView getView(Settings form, boolean reshow, BindException errors)
CustomizeBean bean = new CustomizeBean();

bean.settings = getSettings(); // TODO: Just use form?
bean.returnURL = form.getReturnURLHelper();
bean.returnUrl = form.getReturnUrlHelper();
bean.assignedToSelect = getAssignedToSelect(getContainer(), bean.settings.getDefaultAssignedTo(), "defaultAssignedTo", getUser());

if (hasEditorPerm(Group.groupGuests))
Expand Down Expand Up @@ -751,7 +751,7 @@ public void addNavTrail(NavTree root)
public static class CustomizeBean
{
public Settings settings;
public URLHelper returnURL; // TODO: Settings has a returnUrl
public URLHelper returnUrl; // TODO: Settings has a returnUrl
public String securityWarning;
public SelectBuilder assignedToSelect;
}
Expand Down Expand Up @@ -821,35 +821,35 @@ public boolean handlePost(AnnouncementForm form, BindException errors)
return false;
}

URLHelper returnURL = form.getReturnURLHelper();
URLHelper returnUrl = form.getReturnUrlHelper();

// Null in insert/update message case, since we want to redirect to thread view anchoring to new post
if (null == returnURL)
if (null == returnUrl)
{
AnnouncementModel thread = insert;
if (null != insert.getParent())
thread = AnnouncementManager.getAnnouncement(getContainer(), insert.getParent());

if (form.isFromDiscussion() && null != thread.getDiscussionSrcIdentifier())
{
returnURL = DiscussionServiceImpl.fromSaved(thread.getDiscussionSrcURL());
returnURL.addParameter("discussion.id", "" + thread.getRowId());
returnURL.addParameter("_anchor", "discussionArea"); // TODO: insert.getRowId() instead? -- target just inserted response
returnUrl = DiscussionServiceImpl.fromSaved(thread.getDiscussionSrcURL());
returnUrl.addParameter("discussion.id", "" + thread.getRowId());
returnUrl.addParameter("_anchor", "discussionArea"); // TODO: insert.getRowId() instead? -- target just inserted response
}
else
{
String threadId = thread.getEntityId();
returnURL = getThreadURL(c, threadId, insert.getRowId());
returnUrl = getThreadURL(c, threadId, insert.getRowId());
}
}

_attachmentErrorView = AttachmentService.get().getErrorView(files, errors, returnURL);
_attachmentErrorView = AttachmentService.get().getErrorView(files, errors, returnUrl);

boolean success = (null == _attachmentErrorView);

// Can't use getSuccessURL since this is a URLHelper, not an ActionURL
if (success)
throw new RedirectException(returnURL);
throw new RedirectException(returnUrl);

return false;
}
Expand Down Expand Up @@ -895,7 +895,7 @@ public ModelAndView getInsertUpdateView(AnnouncementForm form, boolean reshow, B
throw new UnauthorizedException();
}

InsertMessageView insertView = new InsertMessageView(form, "New " + settings.getConversationName(), errors, reshow, form.getReturnURLHelper(), false, true);
InsertMessageView insertView = new InsertMessageView(form, "New " + settings.getConversationName(), errors, reshow, form.getReturnUrlHelper(), false, true);
insertView.setShowTitle(false);

getPageConfig().setFocusId("title");
Expand Down Expand Up @@ -956,7 +956,7 @@ public ModelAndView getInsertUpdateView(AnnouncementForm form, boolean reshow, B
ThreadView threadView = new ThreadView(c, getActionURL(), parent, perm);
threadView.setFrame(WebPartView.FrameType.DIV);

HttpView respondView = new RespondView(c, parent, form, form.getReturnURLHelper(), errors, reshow, false);
HttpView respondView = new RespondView(c, parent, form, form.getReturnUrlHelper(), errors, reshow, false);

getPageConfig().setFocusId("body");
_parent = parent;
Expand Down Expand Up @@ -1067,11 +1067,11 @@ else if (!user.isGuest())
}


private static ActionURL getInsertURL(Container c, @Nullable ActionURL returnURL)
private static ActionURL getInsertURL(Container c, @Nullable ActionURL returnUrl)
{
ActionURL url = new ActionURL(InsertAction.class, c);
if (returnURL != null)
url.addReturnURL(returnURL);
if (returnUrl != null)
url.addReturnUrl(returnUrl);

return url;
}
Expand Down Expand Up @@ -1200,7 +1200,7 @@ public static ActionURL getUpdateURL(Container c, String threadId, URLHelper ret
{
ActionURL url = new ActionURL(UpdateAction.class, c);
url.addParameter("entityId", threadId);
url.addReturnURL(returnUrl);
url.addReturnUrl(returnUrl);
return url;
}

Expand Down Expand Up @@ -1288,7 +1288,7 @@ public boolean handlePost(AnnouncementForm form, BindException errors)

// Needs to support non-ActionURL (e.g., an HTML page using the client API with embedded discussion webpart)
// so we can't use getSuccessURL()
URLHelper urlHelper = form.getReturnURLHelper();
URLHelper urlHelper = form.getReturnUrlHelper();
if (null != urlHelper)
throw new RedirectException(urlHelper);
else
Expand Down Expand Up @@ -1496,7 +1496,7 @@ public static ActionURL getEmailPreferencesURL(Container c, @Nullable URLHelper
ActionURL result = new ActionURL(EmailPreferencesAction.class, c);
result.addParameter("srcIdentifier", srcIdentifier);
if (returnUrl != null)
result.addReturnURL(returnUrl);
result.addReturnUrl(returnUrl);

return result;
}
Expand Down Expand Up @@ -2022,13 +2022,13 @@ private static ActionURL getPageURL(ViewContext ctx)
{
// This is set to the outer page URL in the case of rendering a dynamic webpart; use it instead of
// the getWebPart URL.
String returnURL = (String)ctx.get(ActionURL.Param.returnUrl.name());
String returnUrl = (String)ctx.get(ActionURL.Param.returnUrl.name());

if (null != returnURL)
if (null != returnUrl)
{
try
{
return new ActionURL(returnURL);
return new ActionURL(returnUrl);
}
catch (IllegalArgumentException x)
{
Expand Down Expand Up @@ -2371,7 +2371,7 @@ protected void init(Container c, AnnouncementModel ann, URLHelper currentURL, Pe
bean.message = null;
bean.perm = perm;
bean.isResponse = isResponse;
bean.messagesURL = getBeginURL(c); // TODO: Used as returnURL after delete thread... should be messages or list, as appropriate
bean.messagesURL = getBeginURL(c); // TODO: Used as returnUrl after delete thread... should be messages or list, as appropriate
bean.listURL = getListURL(c);
bean.printURL = null == currentURL ? null : currentURL.clone().replaceParameter(ActionURL.Param._print.name(), "1");
bean.print = print;
Expand Down Expand Up @@ -2399,7 +2399,7 @@ protected void init(Container c, AnnouncementModel ann, URLHelper currentURL, Pe
// Build up a link to unsubscribe from the thread
ActionURL url = new ActionURL(SubscribeThreadAction.class, c);
url.addParameter("threadId", ann.getParent() == null ? ann.getEntityId() : ann.getParent());
url.addReturnURL(getViewContext().getActionURL());
url.addReturnUrl(getViewContext().getActionURL());
url.addParameter("unsubscribe", true);
buttons.addChild("unsubscribe", url).usePost();
}
Expand Down Expand Up @@ -2438,7 +2438,7 @@ else if ((emailOption == EmailOption.MESSAGES_MINE.getValue())
subscribeTree.addChild("forum", getEmailPreferencesURL(c, getViewContext().getActionURL(), ann.lookupSrcIdentifier()));
ActionURL subscribeThreadURL = new ActionURL(SubscribeThreadAction.class, c);
subscribeThreadURL.addParameter("threadId", ann.getParent() == null ? ann.getEntityId() : ann.getParent());
subscribeThreadURL.addReturnURL(getViewContext().getActionURL());
subscribeThreadURL.addReturnUrl(getViewContext().getActionURL());
subscribeTree.addChild("thread", subscribeThreadURL).usePost();
buttons.addChild(subscribeTree);
}
Expand Down Expand Up @@ -2582,7 +2582,7 @@ public class UpdateBean
public SelectBuilder statusSelect;
public SelectBuilder renderAsSelect;
public String memberList;
public URLHelper returnURL;
public URLHelper returnUrl;

private UpdateBean(AnnouncementForm form, AnnouncementModel ann)
{
Expand All @@ -2595,7 +2595,7 @@ private UpdateBean(AnnouncementForm form, AnnouncementModel ann)
statusSelect = getStatusSelect(ann.getStatus());
assignedToSelect = getAssignedToSelect(c, ann.getAssignedTo(), "assignedTo", getViewContext().getUser());
renderAsSelect = getRenderAsSelect(WikiRendererType.valueOf(ann.getRendererType()));
returnURL = form.getReturnURLHelper();
returnUrl = form.getReturnUrlHelper();
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions announcements/src/org/labkey/announcements/announcementThread.jsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ if (!bean.isResponse && !bean.print)
else
{
ActionURL respond = announcementURL(c, RespondAction.class, "parentId", announcementModel.getEntityId());
respond.addReturnURL(bean.currentURL);
respond.addReturnUrl(bean.currentURL);
%>
<%= button("Respond").href(respond) %>&nbsp;<%
}
Expand All @@ -282,11 +282,11 @@ if (!bean.isResponse && !bean.print)
if (bean.embedded)
{
URLHelper redirect = bean.currentURL.clone().deleteScopeParameters("discussion");
deleteThread.addReturnURL(redirect);
deleteThread.addReturnUrl(redirect);
}
else
{
deleteThread.addReturnURL(bean.messagesURL);
deleteThread.addReturnUrl(bean.messagesURL);
}
%>
<%= button("Delete " + settings.getConversationName()).href(deleteThread) %>&nbsp;<%
Expand Down
6 changes: 3 additions & 3 deletions announcements/src/org/labkey/announcements/customize.jsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
DiscussionService.Settings settings = bean.settings;

%><labkey:form action="<%=urlFor(CustomizeAction.class)%>" method="post">
<%=generateReturnUrlFormField(bean.returnURL)%>
<%=generateReturnUrlFormField(bean.returnUrl)%>
<table class="lk-fields-table">
<tr>
<td class="labkey-form-label">Board name</td>
Expand Down Expand Up @@ -168,8 +168,8 @@
<td colspan=2>
<br/>
<%= button("Save").submit(true) %>
<% if (null != bean.returnURL) { %>
<%= button("Cancel").href(bean.returnURL) %>
<% if (null != bean.returnUrl) { %>
<%= button("Cancel").href(bean.returnUrl) %>
<% } %>
</td>
</tr>
Expand Down
2 changes: 1 addition & 1 deletion announcements/src/org/labkey/announcements/update.jsp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
<labkey:form method="post" action='<%=baseUrl.setAction(AnnouncementsController.UpdateAction.class)%>' enctype="multipart/form-data" onsubmit="return onSubmit(this);">
<labkey:input type="hidden" name="rowId" value="<%=ann.getRowId()%>"/>
<labkey:input type="hidden" name="entityId" value="<%=ann.getEntityId()%>"/>
<%=generateReturnUrlFormField(bean.returnURL)%>
<%=generateReturnUrlFormField(bean.returnUrl)%>
<table><%

if (settings.isTitleEditable())
Expand Down
53 changes: 5 additions & 48 deletions api/src/org/labkey/api/action/ReturnUrlForm.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,11 @@
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.jetbrains.annotations.Nullable;
import org.labkey.api.settings.AppProps;
import org.labkey.api.util.HtmlString;
import org.labkey.api.util.PageFlowUtil;
import org.labkey.api.util.ReturnURLString;
import org.labkey.api.util.URLHelper;
import org.labkey.api.view.ActionURL;
import org.labkey.api.view.HttpView;

/**
* Simple form bean that includes a returnUrl property, typically used to send the user back to the page where they initiated the action.
Expand Down Expand Up @@ -89,7 +87,7 @@ public String getUrlhash()
}

@Nullable
public URLHelper getReturnURLHelper()
public URLHelper getReturnUrlHelper()
{
return _returnUrl != null ? _returnUrl.getURLHelper() : null;
}
Expand All @@ -111,9 +109,9 @@ public ActionURL getReturnActionURL(ActionURL defaultURL)
/**
* Get the first non-null URL from <code>returnUrl</code> or the <code>defaultURL</code> parameter.
*/
public URLHelper getReturnURLHelper(URLHelper defaultURL)
public URLHelper getReturnUrlHelper(URLHelper defaultURL)
{
return firstOf(getReturnURLHelper(), defaultURL);
return firstOf(getReturnUrlHelper(), defaultURL);
}

/**
Expand Down Expand Up @@ -192,27 +190,12 @@ public ActionURL getSuccessActionURL(ActionURL defaultURL)
defaultURL);
}

// when we convert code to use ReturnUrlForm we may leave behind bookmarks using "returnURL"
@Deprecated
public ReturnURLString getReturnURL()
{
throwBadParam();
return _returnUrl;
}

@Deprecated
public void setReturnURL(ReturnURLString returnUrl)
{
throwBadParam();
setReturnUrl(returnUrl);
}

/** Applies the return URL from this form (if any) to the given URL */
public void propagateReturnURL(ActionURL urlNeedingParameter)
{
if (getReturnURLHelper() != null)
if (getReturnUrlHelper() != null)
{
urlNeedingParameter.addReturnURL(getReturnURLHelper());
urlNeedingParameter.addReturnUrl(getReturnUrlHelper());
}
}

Expand All @@ -225,30 +208,4 @@ private static <X> X firstOf(X... urls)

return null;
}

/**
* Report a bad returnUrl usage.
* Some views don't show Spring binding errors from the thrown exception so
* log an ERROR message to the console and let the test framework report it.
*/
public static void throwBadParam()
{
throwBadParam("returnURL");
}

/**
* Report a bad returnUrl usage.
* Some views don't show Spring binding errors from the thrown exception so
* log an ERROR message to the console and let the test framework report it.
*/
public static void throwBadParam(String badParamName)
{
StringBuilder msg = new StringBuilder("Use 'returnUrl' instead of '").append(badParamName).append("'");
if (HttpView.hasCurrentView())
msg.append(" from URL: ").append(HttpView.currentContext().getRequest().getRequestURI());
LOG.error(msg.toString());
if (AppProps.getInstance().isDevMode())
throw new UnsupportedOperationException(msg.toString());
}

}
Loading