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

#2824 Fixed event handlers new xid not displaying after saving edit form #2959

Conversation

Patrykb0802
Copy link
Contributor

No description provided.

@Patrykb0802 Patrykb0802 requested review from Limraj and SoftQ as code owners July 18, 2024 10:18
Copy link

github-actions bot commented Jul 18, 2024

Java Script Mocha Unit Test Results

268 tests   268 ✅  4s ⏱️
 70 suites    0 💤
  1 files      0 ❌

Results for commit dbd759d.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Jul 18, 2024

Java JUnit Test Results

2 089 tests   2 089 ✅  27s ⏱️
  107 suites      0 💤
  107 files        0 ❌

Results for commit dbd759d.

♻️ This comment has been updated with latest results.

@@ -508,6 +508,7 @@

selectedHandlerNode.object = handler;
}
$set("xid", handler.xid);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where is the definition of the handler variable?
What is defined in the else block is not available outside this block.
If the xid has not been completed when editing the handler, it should not save the newly generated xid and return the old xid. Otherwise, this is counterintuitive behavior and the user may unknowingly change the xid of the handler.

- Added isNotEmpty() to common.js
- Moved validateValue() to common.js
- Added restoring previous XID value for Event Handlers when XID field is left empty when saving Event Handler
- Added isNotEmpty() to common.js
- Moved validateValue() to common.js
- Added restoring previous XID value for Event Handlers when XID field is left empty when saving Event Handler
var messages = [];
if (!(isNotEmpty(xid)) && handlerId !== ${NEW_ID}){
validateValue("xid", "<fmt:message key='validate.valueRestored'/>", isNotEmpty, xid, messages)
$set("xid", selectedHandlerNode.object.xid);
Copy link
Collaborator

Choose a reason for hiding this comment

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

added lines:

stopImageFader("saveImg");
showDwrMessages(message);
return;

validateValue("xid", "<fmt:message key='validate.valueRestored'/>", isNotEmpty, xid, messages)
$set("xid", selectedHandlerNode.object.xid);
}
if(messages.length > 0){
Copy link
Collaborator

Choose a reason for hiding this comment

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

if to delete

@@ -2989,6 +2989,7 @@ validate.required=Required value
validate.text.incompatible=Text renderer is incompatible with data type
validate.event.incompatible=Event text renderer is incompatible with data type
validate.xidUsed=This XID is already in use
validate.valueRestored=Previous value restored
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok.
Need to update for each translation files.

@@ -456,36 +456,43 @@
var xid = $get("xid");
var alias = $get("alias");
var disabled = $get("disabled");
if (handlerType == <c:out value="<%= EventHandlerVO.TYPE_EMAIL %>"/>) {
var messages = [];
if (!(isNotEmpty(xid)) && handlerId !== ${NEW_ID}){
Copy link
Collaborator

Choose a reason for hiding this comment

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

!(isNotEmpty(xid)) - Instead of so many negations, it is worth creating the isEmpty method.

if (handlerType == <c:out value="<%= EventHandlerVO.TYPE_EMAIL %>"/>) {
var messages = [];
if (!(isNotEmpty(xid)) && handlerId !== ${NEW_ID}){
validateValue("xid", "<fmt:message key='validate.valueRestored'/>", isNotEmpty, xid, messages)
Copy link
Collaborator

@Limraj Limraj Jul 22, 2024

Choose a reason for hiding this comment

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

Remove use validateValue:
let message = createValidationMessage("xid", "<fmt:message key='validate.valueRestored'/>");

@@ -456,36 +456,43 @@
var xid = $get("xid");
var alias = $get("alias");
var disabled = $get("disabled");
if (handlerType == <c:out value="<%= EventHandlerVO.TYPE_EMAIL %>"/>) {
var messages = [];
Copy link
Collaborator

Choose a reason for hiding this comment

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

messages to delete

@@ -325,7 +325,12 @@ private DwrResponseI18n save(int eventSourceId, int eventTypeRef1,
EventService eventService = new EventService();

vo.setId(handlerId);
vo.setXid(StringUtils.isEmpty(xid) ? eventService.generateUniqueXid() : xid);
if (StringUtils.isEmpty(xid) && handlerId == Common.NEW_ID) {
vo.setXid(StringUtils.isEmpty(xid) ? eventService.generateUniqueXid() : xid);
Copy link
Collaborator

Choose a reason for hiding this comment

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

vo.setXid(eventService.generateUniqueXid());

@@ -2948,6 +2948,7 @@ validate.required=Se requiere un valor
validate.text.incompatible=El generador de Texto es incompatibles con el tipo de dato
validate.event.incompatible=Event text renderer is incompatible with data type
validate.xidUsed=Este XID ya est\u00e1 en uso
validate.valueRestored=Previous value restored
Copy link
Collaborator

Choose a reason for hiding this comment

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

You need to add this at the end of the file, not in the middle.

let message = createValidationMessage("xid", "<fmt:message key='validate.valueRestored'/>");
$set("xid", selectedHandlerNode.object.xid);
stopImageFader("saveImg");
showDwrMessages([message]);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add line:
return;

stopImageFader("saveImg");
showDwrMessages([message]);
}
else {
Copy link
Collaborator

@Limraj Limraj Jul 23, 2024

Choose a reason for hiding this comment

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

What comes after ifa needs to be restored.
You only add a new if at the end of return, we do not make any other changes.

@Limraj Limraj added this to the 2.8.0 milestone Jul 23, 2024
@Patrykb0802 Patrykb0802 requested a review from Limraj July 24, 2024 09:41
Copy link
Collaborator

@Limraj Limraj left a comment

Choose a reason for hiding this comment

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

If we create a new handler and leave the xid field empty, a new handler is created with the generated xid, if we try to save an existing event handler with an empty xid field, we get the currently saved xid with information 'Previous value restored' -> ok.

@Limraj Limraj merged commit 7b25463 into release/2.8.0 Jul 24, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants