PAYARA-1486 Changed the roomName attribute to be roomID #1410
Conversation
|
jenkins test please |
| 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" |
There was a problem hiding this comment.
This won't work - The property name is separate to the parameter name. See the following comment...
| @@ -84,7 +84,7 @@ | |||
| @Param(name = "securityDisabled", defaultValue = "false", optional = true) | |||
| private Boolean securityDisabled; | |||
|
|
|||
| @Param(name = "roomName") | |||
| @Param(name = "roomID") | |||
| private String roomName; | |||
There was a problem hiding this comment.
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
There was a problem hiding this comment.
ah, I thought I could get away with just renaming the bits the user could see >.<
|
One or more tests have failed |
|
Should I close this without merging? |
|
It's a valid PR, it just needs the extra steps I noted in my review completed before we can merge. |
|
PR#1438 makes this one obsolete |
I changed the Admin Console label, asadmin command and the attribute that the admin console uses for the REST API.
I did a simple test to make sure it worked...
both times the roomID was correctly used.