Skip to content

Commit 4ace688

Browse files
yan-3005anuj-kumary
yan-3005
and
anuj-kumary
committed
feat: show proposed changes with clickable entity links in approval tasks (#27201)
* feat: show proposed changes with clickable entity links in approval tasks Governance approval tasks now display a "Proposed Changes" preview so approvers can see exactly what changed before approving. Backend runs the entity's changeDescription through the activity-feed formatter pipeline and stores a markdown bullet list in thread.message. UI renders the preview using Showdown + dangerouslySetInnerHTML and injects clickable hyperlinks for tags, tier, glossary terms, related terms, domains, owners, reviewers, and experts directly in the frontend before conversion. * fix: drop owner links (user/team ambiguous), escape FQN HTML, widen diff-span regex * feat(governance): structured JSON proposed-changes preview in approval tasks - Store thread.message as JSON {field: {added: [...], removed: [...]}} instead of markdown+HTML - Extract identifiers via tagFQN > fullyQualifiedName > displayName > name priority - Merge across re-edits using set-cancellation (items that cancel to empty are dropped) - Extract all change-preview logic into ChangePreviewUtils (governance/workflows/util) - CreateApprovalTaskImpl reduced to pure Flowable orchestration - UI: replace dangerouslySetInnerHTML with structured chip rendering (green=added, red=removed) - Notification panel: replace raw task.message with static approval-request label - 21 unit tests covering extractIdentifiers, buildChangeMap, mergeChangeMaps, parseChangeMap * feat(governance): strikethrough on removed chips in proposed changes * fix(governance): null-safe field list iteration and correct JSON string decoding in ChangePreviewUtils * fix(governance): non-null thread.message fallback and single parseProposedChanges call * fix(governance): O(n) set operations and resilient threshold parsing * i18n: translate proposed-change-plural key in all 18 locale files * Addressed comments * fix lint issue * Fix Playwright: use dedicated i18n key for approval notification, fallback to task.message for non-JSON * Fix review: handle List/Map inputs in extractIdentifiers, gate proposed changes on task type, fix prototype pollution * Fix: add trailing space to approval notification i18n key * Fix review: empty Map/List guard, startCase field labels, chip keys with index, remove CSS capitalize, explicit JSX space * refactor(workflows): simplify ChangePreviewUtils — single JSON path, FieldDiff record, descriptive variable names * fix(workflows): null-safe FieldDiff compact constructor, zh-CN locale for approval notification * i18n: translate request-approval-notification for all 17 locales --------- Co-authored-by: anuj-kumary <anujf0510@gmail.com>
1 parent cc08f80 commit 4ace688

25 files changed

Lines changed: 755 additions & 35 deletions

File tree

‎openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/elements/nodes/userTask/impl/CreateApprovalTaskImpl.java‎

Lines changed: 22 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,15 @@
3232
import org.openmetadata.service.exception.EntityNotFoundException;
3333
import org.openmetadata.service.governance.workflows.WorkflowHandler;
3434
import org.openmetadata.service.governance.workflows.WorkflowVariableHandler;
35+
import org.openmetadata.service.governance.workflows.util.ChangePreviewUtils;
3536
import org.openmetadata.service.jdbi3.FeedRepository;
3637
import org.openmetadata.service.resources.feeds.FeedMapper;
3738
import org.openmetadata.service.resources.feeds.MessageParser;
3839
import org.openmetadata.service.util.WebsocketNotificationHandler;
3940

4041
@Slf4j
4142
public class CreateApprovalTaskImpl implements TaskListener {
43+
4244
private Expression inputNamespaceMapExpr;
4345
private Expression approvalThresholdExpr;
4446
private Expression rejectionThresholdExpr;
@@ -57,31 +59,14 @@ public void notify(DelegateTask delegateTask) {
5759
inputNamespaceMap.get(RELATED_ENTITY_VARIABLE), RELATED_ENTITY_VARIABLE));
5860
EntityInterface entity = Entity.getEntity(entityLink, "*", Include.ALL);
5961

60-
// Get approval threshold, default to 1 if not set
61-
Integer approvalThreshold = 1;
62-
if (approvalThresholdExpr != null) {
63-
String thresholdStr = (String) approvalThresholdExpr.getValue(delegateTask);
64-
if (thresholdStr != null && !thresholdStr.isEmpty()) {
65-
approvalThreshold = Integer.parseInt(thresholdStr);
66-
}
67-
}
68-
69-
// Get rejection threshold, default to 1 if not set
70-
Integer rejectionThreshold = 1;
71-
if (rejectionThresholdExpr != null) {
72-
String thresholdStr = (String) rejectionThresholdExpr.getValue(delegateTask);
73-
if (thresholdStr != null && !thresholdStr.isEmpty()) {
74-
rejectionThreshold = Integer.parseInt(thresholdStr);
75-
}
76-
}
62+
int approvalThreshold = resolveThreshold(approvalThresholdExpr, delegateTask, 1);
63+
int rejectionThreshold = resolveThreshold(rejectionThresholdExpr, delegateTask, 1);
7764

7865
Thread task = createApprovalTask(entity, assignees);
7966
WorkflowHandler.getInstance().setCustomTaskId(delegateTask.getId(), task.getId());
8067

81-
// Set the thresholds as task variables for use in WorkflowHandler
8268
delegateTask.setVariable("approvalThreshold", approvalThreshold);
8369
delegateTask.setVariable("rejectionThreshold", rejectionThreshold);
84-
// Use separate lists for approvers and rejecters - simpler and cleaner
8570
delegateTask.setVariable("approversList", new ArrayList<String>());
8671
delegateTask.setVariable("rejectersList", new ArrayList<String>());
8772
} catch (Exception exc) {
@@ -95,9 +80,24 @@ public void notify(DelegateTask delegateTask) {
9580
}
9681
}
9782

83+
private static int resolveThreshold(
84+
Expression expr, DelegateTask delegateTask, int defaultValue) {
85+
if (expr == null) return defaultValue;
86+
String raw = (String) expr.getValue(delegateTask);
87+
if (raw == null || raw.trim().isEmpty()) return defaultValue;
88+
try {
89+
return Integer.parseInt(raw.trim());
90+
} catch (NumberFormatException exc) {
91+
LOG.warn(
92+
"Invalid threshold value '{}' resolved from workflow expression. Falling back to default value {}.",
93+
raw,
94+
defaultValue);
95+
return defaultValue;
96+
}
97+
}
98+
9899
private List<EntityReference> getAssignees(DelegateTask delegateTask) {
99100
List<EntityReference> assignees = new ArrayList<>();
100-
101101
Set<IdentityLink> candidates = delegateTask.getCandidates();
102102
if (!candidates.isEmpty()) {
103103
for (IdentityLink candidate : candidates) {
@@ -122,22 +122,17 @@ private Thread createApprovalTask(EntityInterface entity, List<EntityReference>
122122
Entity.getEntityTypeFromObject(entity), entity.getFullyQualifiedName());
123123

124124
Thread thread;
125-
126125
ChangeEvent changeEvent;
127126
try {
128127
thread = feedRepository.getTask(about, TaskType.RequestApproval, TaskStatus.Open);
129-
// Update the existing thread with new assignees before terminating the workflow
130128
thread.getTask().setAssignees(FeedMapper.formatAssignees(assignees));
131-
129+
ChangePreviewUtils.applyChangePreview(thread, entity, thread.getMessage());
132130
thread.withUpdatedBy(entity.getUpdatedBy()).withUpdatedAt(System.currentTimeMillis());
133131

134-
// Save the updated thread to database
135132
Entity.getCollectionDAO().feedDAO().update(thread.getId(), JsonUtils.pojoToJson(thread));
136133

137-
// Now terminate the old workflow instance
138134
WorkflowHandler.getInstance()
139135
.terminateTaskProcessInstance(thread.getId(), "A Newer Process Instance is Running.");
140-
// Create and publish ChangeEvent for notification system
141136
changeEvent =
142137
new ChangeEvent()
143138
.withId(UUID.randomUUID())
@@ -158,16 +153,15 @@ private Thread createApprovalTask(EntityInterface entity, List<EntityReference>
158153
new Thread()
159154
.withId(UUID.randomUUID())
160155
.withThreadTs(System.currentTimeMillis())
161-
.withMessage("Approval required for ")
162156
.withCreatedBy(entity.getUpdatedBy())
163157
.withAbout(about.getLinkString())
164158
.withType(ThreadType.Task)
165159
.withTask(taskDetails)
166160
.withUpdatedBy(entity.getUpdatedBy())
167161
.withUpdatedAt(System.currentTimeMillis());
162+
ChangePreviewUtils.applyChangePreview(thread, entity, null);
168163
feedRepository.create(thread);
169164

170-
// Create and publish ChangeEvent for notification system
171165
changeEvent =
172166
new ChangeEvent()
173167
.withId(UUID.randomUUID())
@@ -179,7 +173,6 @@ private Thread createApprovalTask(EntityInterface entity, List<EntityReference>
179173
.withEntity(thread);
180174
}
181175
Entity.getCollectionDAO().changeEventDAO().insert(JsonUtils.pojoToMaskedJson(changeEvent));
182-
// Send WebSocket Notification
183176
WebsocketNotificationHandler.handleTaskNotification(thread);
184177
return thread;
185178
}

‎openmetadata-service/src/main/java/org/openmetadata/service/governance/workflows/util/ChangePreviewUtils.java‎

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
/*
2+
* Copyright 2021 Collate
3+
* Licensed under the Apache License, Version 2.0 (the "License");
4+
* you may not use this file except in compliance with the License.
5+
* You may obtain a copy of the License at
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
* Unless required by applicable law or agreed to in writing, software
8+
* distributed under the License is distributed on an "AS IS" BASIS,
9+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
10+
* See the License for the specific language governing permissions and
11+
* limitations under the License.
12+
*/
13+
14+
package org.openmetadata.service.governance.workflows.util;
15+
16+
import static org.openmetadata.common.utils.CommonUtil.listOrEmpty;
17+
import static org.openmetadata.common.utils.CommonUtil.nullOrEmpty;
18+
19+
import com.fasterxml.jackson.core.type.TypeReference;
20+
import java.util.Collection;
21+
import java.util.HashSet;
22+
import java.util.LinkedHashMap;
23+
import java.util.List;
24+
import java.util.Map;
25+
import java.util.Set;
26+
import java.util.stream.Stream;
27+
import lombok.extern.slf4j.Slf4j;
28+
import org.openmetadata.schema.EntityInterface;
29+
import org.openmetadata.schema.entity.feed.Thread;
30+
import org.openmetadata.schema.type.ChangeDescription;
31+
import org.openmetadata.schema.type.FieldChange;
32+
import org.openmetadata.schema.utils.JsonUtils;
33+
34+
/**
35+
* Builds and merges structured change-preview JSON stored in {@code thread.message} for approval
36+
* tasks. The message format is a JSON object mapping field names to {@code {added, removed}} arrays
37+
* of human-readable identifiers (tagFQN, fullyQualifiedName, displayName, or name).
38+
*/
39+
@Slf4j
40+
public final class ChangePreviewUtils {
41+
42+
private static final List<String> ID_KEYS =
43+
List.of("tagFQN", "fullyQualifiedName", "displayName", "name");
44+
45+
private static final TypeReference<Map<String, FieldDiff>> CHANGE_MAP_TYPE =
46+
new TypeReference<>() {};
47+
48+
private ChangePreviewUtils() {}
49+
50+
public record FieldDiff(List<String> added, List<String> removed) {
51+
public FieldDiff {
52+
added = added != null ? added : List.of();
53+
removed = removed != null ? removed : List.of();
54+
}
55+
56+
FieldDiff merge(FieldDiff next) {
57+
return new FieldDiff(
58+
union(minus(added, next.removed), minus(next.added, removed)),
59+
union(minus(removed, next.added), minus(next.removed, added)));
60+
}
61+
62+
boolean isEmpty() {
63+
return added.isEmpty() && removed.isEmpty();
64+
}
65+
}
66+
67+
public static List<String> extractIdentifiers(Object value) {
68+
return collect(normalize(value));
69+
}
70+
71+
private static Object normalize(Object value) {
72+
if (!(value instanceof String raw)) return value;
73+
String stripped = raw.strip();
74+
if (stripped.isEmpty()) return null;
75+
try {
76+
return JsonUtils.readValue(stripped, Object.class);
77+
} catch (Exception e) {
78+
return stripped;
79+
}
80+
}
81+
82+
private static List<String> collect(Object value) {
83+
if (value == null) return List.of();
84+
if (value instanceof Collection<?> collection) {
85+
return collection.stream().flatMap(item -> collect(item).stream()).toList();
86+
}
87+
if (value instanceof Map<?, ?> map) {
88+
for (String key : ID_KEYS) {
89+
if (map.get(key) instanceof String idValue && !idValue.isBlank())
90+
return List.of(idValue.strip());
91+
}
92+
List<String> nested =
93+
map.values().stream()
94+
.filter(item -> item instanceof Map || item instanceof Collection)
95+
.flatMap(item -> collect(item).stream())
96+
.toList();
97+
return nested.isEmpty() ? List.of(JsonUtils.pojoToJson(map)) : nested;
98+
}
99+
String str = value.toString().strip();
100+
return str.isEmpty() ? List.of() : List.of(str);
101+
}
102+
103+
private static List<String> union(List<String> left, List<String> right) {
104+
return Stream.concat(left.stream(), right.stream()).distinct().toList();
105+
}
106+
107+
private static List<String> minus(List<String> source, Collection<String> exclusions) {
108+
Set<String> exclusionSet = new HashSet<>(exclusions);
109+
return source.stream().filter(item -> !exclusionSet.contains(item)).toList();
110+
}
111+
112+
public static Map<String, FieldDiff> buildChangeMap(ChangeDescription changeDescription) {
113+
Map<String, FieldDiff> result = new LinkedHashMap<>();
114+
for (FieldChange fieldChange : listOrEmpty(changeDescription.getFieldsAdded())) {
115+
result.put(
116+
fieldChange.getName(),
117+
new FieldDiff(extractIdentifiers(fieldChange.getNewValue()), List.of()));
118+
}
119+
for (FieldChange fieldChange : listOrEmpty(changeDescription.getFieldsDeleted())) {
120+
result.put(
121+
fieldChange.getName(),
122+
new FieldDiff(List.of(), extractIdentifiers(fieldChange.getOldValue())));
123+
}
124+
for (FieldChange fieldChange : listOrEmpty(changeDescription.getFieldsUpdated())) {
125+
result.put(
126+
fieldChange.getName(),
127+
new FieldDiff(
128+
extractIdentifiers(fieldChange.getNewValue()),
129+
extractIdentifiers(fieldChange.getOldValue())));
130+
}
131+
return result;
132+
}
133+
134+
public static Map<String, FieldDiff> mergeChangeMaps(
135+
Map<String, FieldDiff> oldMap, Map<String, FieldDiff> newMap) {
136+
Map<String, FieldDiff> merged = new LinkedHashMap<>(oldMap);
137+
for (Map.Entry<String, FieldDiff> entry : newMap.entrySet()) {
138+
String field = entry.getKey();
139+
merged.put(
140+
field,
141+
merged.containsKey(field) ? merged.get(field).merge(entry.getValue()) : entry.getValue());
142+
}
143+
merged.entrySet().removeIf(entry -> entry.getValue().isEmpty());
144+
return merged;
145+
}
146+
147+
public static boolean hasNoChanges(ChangeDescription changeDescription) {
148+
return changeDescription == null
149+
|| (nullOrEmpty(changeDescription.getFieldsAdded())
150+
&& nullOrEmpty(changeDescription.getFieldsUpdated())
151+
&& nullOrEmpty(changeDescription.getFieldsDeleted()));
152+
}
153+
154+
public static Map<String, FieldDiff> parseChangeMap(String message) {
155+
if (nullOrEmpty(message) || !message.strip().startsWith("{")) return new LinkedHashMap<>();
156+
try {
157+
return JsonUtils.readValue(message, CHANGE_MAP_TYPE);
158+
} catch (Exception e) {
159+
return new LinkedHashMap<>();
160+
}
161+
}
162+
163+
public static void applyChangePreview(
164+
Thread taskThread, EntityInterface entity, String oldMessage) {
165+
taskThread.withCardStyle(null).withFieldOperation(null).withFeedInfo(null);
166+
final ChangeDescription changeDescription = entity.getChangeDescription();
167+
if (hasNoChanges(changeDescription)) {
168+
taskThread.withMessage(oldMessage != null ? oldMessage : "{}");
169+
return;
170+
}
171+
try {
172+
Map<String, FieldDiff> merged =
173+
mergeChangeMaps(parseChangeMap(oldMessage), buildChangeMap(changeDescription));
174+
taskThread.withMessage(merged.isEmpty() ? "{}" : JsonUtils.pojoToJson(merged));
175+
} catch (Exception e) {
176+
LOG.warn(
177+
"Failed to build change preview for approval task on {}",
178+
entity.getFullyQualifiedName(),
179+
e);
180+
taskThread.withMessage(oldMessage != null ? oldMessage : "{}");
181+
}
182+
}
183+
}

0 commit comments

Comments
 (0)