Skip to content
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.

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

PAYARA-1486 Changed the roomName attribute to be roomID #1410

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
notifier.xmpp.tabs.tabText=XMPP
notifier.xmpp.tabs.tabToolTip=XMPP Notification Configuration
notifier.xmpp.configuration.pageTitle=XMPP Notifier Configuration
notifier.xmpp.configuration.pageHelpText=Enable and configure a XMPP notifier.
notifier.xmpp.configuration.pageHelpText=Enable and configure an XMPP notifier.
notifier.xmpp.configuration.enabledLabel=Enabled
notifier.xmpp.configuration.enabledLabelHelpText=Enables/Disables the XMPP notifier.
notifier.xmpp.configuration.hostNameLabel=Host
Expand All @@ -34,5 +34,5 @@ notifier.xmpp.configuration.passwordLabel=Password
notifier.xmpp.configuration.passwordLabelHelpText=Password to use.
notifier.xmpp.configuration.securityDisabledLabel=Security Disabled
notifier.xmpp.configuration.securityDisabledLabelHelpText=Check if you wish to disable security.
notifier.xmpp.configuration.roomNameLabel=Room Name
notifier.xmpp.configuration.roomNameLabelHelpText=Name of the room to use.
notifier.xmpp.configuration.roomIDLabel=Room ID
notifier.xmpp.configuration.roomIDLabelHelpText=The ID of the room to send notifications to.
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,11 @@
<sun:checkbox id="securityDisabledBox" selected="#{pageSession.securityDisabledSelected}"
selectedValue="true" />
</sun:property>
<sun:property id="roomNameProp" labelAlign="left" noWrap="#{true}" overlapLabel="#{false}"
label="$resource{i18nxn.notifier.xmpp.configuration.roomNameLabel}"
helpText="$resource{i18nxn.notifier.xmpp.configuration.roomNameLabelHelpText}">
<sun:textField id="roomName" columns="$int{75}" maxLength="255"
text="#{pageSession.valueMap['roomName']}" styleClass="required"
<sun:property id="roomIDProp" labelAlign="left" noWrap="#{true}" overlapLabel="#{false}"
label="$resource{i18nxn.notifier.xmpp.configuration.roomIDLabel}"
helpText="$resource{i18nxn.notifier.xmpp.configuration.roomIDLabelHelpText}">
<sun:textField id="roomID" columns="$int{75}" maxLength="255"
text="#{pageSession.valueMap['roomID']}" styleClass="required"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work - The property name is separate to the parameter name. See the following comment...

required="#{true}"/>
</sun:property>
</sun:propertySheetSection>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public class XmppNotificationConfigurer extends BaseNotificationConfigurer<XmppN
@Param(name = "securityDisabled", defaultValue = "false", optional = true)
private Boolean securityDisabled;

@Param(name = "roomName")
@Param(name = "roomID")
private String roomName;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bleurgh! Refactoring just the parameter name isn't very nice! 😝
It's not just messy - as I mentioned in the previous comment, the property returned by the get command is separate to this parameter name, so you still have some refactoring to do!

A quick grep (and off the top of my head) you'll need to refactor all occurrences of roomName to roomID:

  • In this class
  • In the XmppNotifierConfiguration class
  • In the GetXmppNotifierConfiguration class
  • In the XmppNotifierConfigurationExecutionOptions class

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, I thought I could get away with just renaming the bits the user could see >.<


protected void applyValues(XmppNotifierConfiguration configuration) throws PropertyVetoException {
Expand Down Expand Up @@ -124,4 +124,4 @@ protected String getHealthCheckNotifierCommandName() {
protected String getRequestTracingNotifierCommandName() {
return "requesttracing-xmpp-notifier-configure";
}
}
}