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

[RHPAM-4445] Unable to update the task description with long string(m… #2201

Merged
merged 2 commits into from Aug 23, 2022

Conversation

abhijithumbe
Copy link
Contributor

…ore than 255 chars)

JIRA: (https://issues.redhat.com/browse/JBPM-10113)

@kie-ci
Copy link

kie-ci commented Aug 10, 2022

Can one of the admins verify this patch?

11 similar comments
@kie-ci
Copy link

kie-ci commented Aug 10, 2022

Can one of the admins verify this patch?

@kie-ci
Copy link

kie-ci commented Aug 10, 2022

Can one of the admins verify this patch?

@kie-ci
Copy link

kie-ci commented Aug 10, 2022

Can one of the admins verify this patch?

@kie-ci
Copy link

kie-ci commented Aug 10, 2022

Can one of the admins verify this patch?

@kie-ci
Copy link

kie-ci commented Aug 10, 2022

Can one of the admins verify this patch?

@kie-ci
Copy link

kie-ci commented Aug 10, 2022

Can one of the admins verify this patch?

@kie-ci
Copy link

kie-ci commented Aug 10, 2022

Can one of the admins verify this patch?

@kie-ci
Copy link

kie-ci commented Aug 10, 2022

Can one of the admins verify this patch?

@kie-ci
Copy link

kie-ci commented Aug 10, 2022

Can one of the admins verify this patch?

@kie-ci
Copy link

kie-ci commented Aug 10, 2022

Can one of the admins verify this patch?

@kie-ci
Copy link

kie-ci commented Aug 10, 2022

Can one of the admins verify this patch?

String message = getUpdateFieldLog("Description", auditTaskImpl.getDescription(), ti.getDescription());

if (message != null && message.length() > TASK_DESCRIPTION_LENGTH) {
Copy link
Member

Choose a reason for hiding this comment

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

this should not be required. it should be trimmed already in the task.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @elguardian, TaskEvent message is getting persisted in the TaskEvent table and here task event message build as "Updated Description from: 'old value' to:new value' ". If new value of task description is 255 chars, overall task event message length is more than 255 chars so we have to trim the message as well.

Copy link
Member

Choose a reason for hiding this comment

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

that is the reason it should be done in some other place. you will have a description that won't match database.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@elguardian I agree task description in TaskEvent wont match with task description in other tables, but message field from TaskEvent is not used anywhere else and actual value of TaskDescription is captured and in sync in Task and AuditTaskImpl table. If we dont update the TaskEvent.message then we have to trim the taskDescription more than defined length. I can modify warn message and capture the task event message before truncating.

Copy link
Member

Choose a reason for hiding this comment

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

@abhijithumbe task event is used in emitters... anycase there is much time for the cut off. I will let this slip

Copy link
Contributor

@afalhambra afalhambra left a comment

Choose a reason for hiding this comment

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

Hi @abhijithumbe, can you please elaborate a little more in the jira ticket. Seeing the PR is not clear to me about the scope, since the DDL scripts are fixed and limiting the length of this field to 255 chars. So even if customer set this new property to a value higher than 255, it will keep on failing if the description is longer than 255 characters.

private static final Logger logger = LoggerFactory.getLogger(TaskImpl.class);

@Transient
private final int TASK_DESCRIPTION_LENGTH = Integer.parseInt(System.getProperty("org.jbpm.ht.task.description.length", "255"));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private final int TASK_DESCRIPTION_LENGTH = Integer.parseInt(System.getProperty("org.jbpm.ht.task.description.length", "255"));
private static final int TASK_DESCRIPTION_LENGTH = Integer.parseInt(System.getProperty("org.jbpm.ht.task.description.length", "255"));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @afalhambra Values defined for system-property and DB column size should be in sync. We need to use Alter table query to modify DB column size based on value defined for system-property

@@ -63,6 +63,8 @@ public class JPATaskLifeCycleEventListener extends PersistableEventListener impl
private static final Logger logger = LoggerFactory.getLogger(JPATaskLifeCycleEventListener.class);
private List<ArchiveLoggerProvider> archiveLoggerProviders;

private final int TASK_DESCRIPTION_LENGTH = Integer.parseInt(System.getProperty("org.jbpm.ht.task.description.length", "255"));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private final int TASK_DESCRIPTION_LENGTH = Integer.parseInt(System.getProperty("org.jbpm.ht.task.description.length", "255"));
private static final int TASK_DESCRIPTION_LENGTH = Integer.parseInt(System.getProperty("org.jbpm.ht.task.description.length", "255"));

@abhijithumbe abhijithumbe requested review from afalhambra and elguardian and removed request for afalhambra and elguardian August 18, 2022 03:23
@elguardian
Copy link
Member

Jenkins retest this please

@sonarcloud
Copy link

sonarcloud bot commented Aug 22, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

94.1% 94.1% Coverage
0.0% 0.0% Duplication

@elguardian elguardian merged commit 194fa05 into kiegroup:main Aug 23, 2022
@elguardian
Copy link
Member

@abhijithumbe plz backport this to7.67 and 7.67-blue

@abhijithumbe
Copy link
Contributor Author

Thanks @elguardian . Opened PR for #2207 and #2208 for 7.67 and 7.67-blue branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants