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

feat: highlight diffs for properties #5282

Merged
merged 9 commits into from
Nov 25, 2024
Merged
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
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Apollo 2.4.0
* [Feature: Added the value length limit function for AppId-level configuration items](https://github.com/apolloconfig/apollo/pull/5264)
* [Fix: ensure clusters order in envClusters open api](https://github.com/apolloconfig/apollo/pull/5277)
* [Fix: bump xstream from 1.4.20 to 1.4.21 to fix CVE-2024-47072](https://github.com/apolloconfig/apollo/pull/5280)
* [Feature: highlight diffs for properties](https://github.com/apolloconfig/apollo/pull/5282)

------------------
All issues and pull requests are [here](https://github.com/apolloconfig/apollo/milestone/15?closed=1)
27 changes: 17 additions & 10 deletions apollo-portal/src/main/resources/static/config/diff.html
Original file line number Diff line number Diff line change
Expand Up @@ -127,31 +127,38 @@ <h4 class="modal-title">{{'Config.Diff.Title' | translate }}
</label>
</div>
<div class="col-sm-12 diff-content">
<!--表格对比 properties mode-->
<table class="table table-bordered table-striped table-hover" ng-show="isTableDiff">
<thead>
<thead>
<tr>
<td>Key</td>
<td ng-repeat="cluster in syncData.syncToNamespaces">
<th>Key</th>
<th ng-repeat="cluster in syncData.syncToNamespaces">
Value ( {{'Common.Environment' | translate }} : <label ng-bind="cluster.env"></label> , {{'Common.Cluster' | translate }} : <label ng-bind="cluster.clusterName"></label> )
</td>
<td ng-show="showCommentDiff" ng-repeat="cluster in syncData.syncToNamespaces">
</th>
<th ng-show="showCommentDiff" ng-repeat="cluster in syncData.syncToNamespaces">
Comment ( {{'Common.Environment' | translate }} : <label ng-bind="cluster.env"></label> , {{'Common.Cluster' | translate }} : <label ng-bind="cluster.clusterName"></label> )
</td>
</th>
</tr>
</thead>
<tbody>
<tr ng-repeat="(key, itemsKeyedByCluster) in itemsKeyedByKey" ng-if="(searchKey === '' || key.indexOf(searchKey) !== -1 ) && (!onlyShowDiffKeys || (onlyShowDiffKeys && !allNamespaceValueEqualed[key]))">
<td width="15%" ng-bind="key"></td>
<td ng-repeat="cluster in syncData.syncToNamespaces"
ng-bind="(itemsKeyedByCluster[cluster.env + ':' + cluster.clusterName + ':' + cluster.namespaceName] || {}).value">
<td ng-bind="(itemsKeyedByCluster[syncData.firstClusterKey] || {}).value"></td>
<td ng-repeat="cluster in syncData.syncToNamespaces.slice(1)" class="cursor-pointer"
ng-click="showTextDiff((itemsKeyedByCluster[syncData.firstClusterKey] || {}).value, (itemsKeyedByCluster[cluster.compositedKey] || {}).value)">
<span ng-if="itemsKeyedByCluster[cluster.compositedKey]"
ng-bind="(itemsKeyedByCluster[cluster.compositedKey] || {}).value">
</span>
</td>
<td ng-show="showCommentDiff"
ng-repeat="cluster in syncData.syncToNamespaces"
ng-bind="(itemsKeyedByCluster[cluster.env + ':' + cluster.clusterName + ':' + cluster.namespaceName] || {}).comment">
ng-bind="(itemsKeyedByCluster[cluster.compositedKey] || {}).comment">
</td>
</tr>
</tbody>
</table>

<!--纯文本对比 text mode-->
<apollodiff ng-show="!isTableDiff" old-str="syncData.syncToNamespaces[0].originTextInfo"
new-str="syncData.syncToNamespaces[1].originTextInfo" apollo-id="'releaseStrDiff'">
</apollodiff>
Expand All @@ -162,7 +169,7 @@ <h4 class="modal-title">{{'Config.Diff.Title' | translate }}
</div>
</section>

<showtextmodal text="text" />
<showtextmodal enable-text-diff='1' old-str="oldStr" new-str="newStr"/>
</div>


Expand Down
13 changes: 6 additions & 7 deletions apollo-portal/src/main/resources/static/config/history.html
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,11 @@ <h6 class="col-md-5 text-right" ng-bind="releaseHistory.releaseTimeFormatted"></
<section ng-show="history.viewType=='diff'">
<h4 class="section-title">{{'Config.History.ChangedItem' | translate }}</h4>
<div ng-show="history.changes && history.changes.length > 0">
<table class="no-margin table table-striped table-hover table-bordered">
<table class="no-margin table table-striped table-hover table-bordered table-fixed">
<thead>
<tr>
<th>{{'Config.History.ChangeType' | translate }}</th>
<th>{{'Config.History.ChangeKey' | translate }}</th>
<th width="10%">{{'Config.History.ChangeType' | translate }}</th>
<th width="12%">{{'Config.History.ChangeKey' | translate }}</th>
<th>{{'Config.History.ChangeOldValue' | translate }}</th>
<th>{{'Config.History.ChangeNewValue' | translate }}</th>
</tr>
Expand Down Expand Up @@ -189,10 +189,9 @@ <h4 class="section-title">{{'Config.History.ChangedItem' | translate }}</h4>
ng-bind="change.entity.firstEntity.value.length > 250 ? '...' :''"></span>
</td>
<td class="cursor-pointer" width="35%"
ng-click="showText(change.entity.secondEntity.value)">
ng-click="showTextDiff(change.entity.firstEntity.value, change.entity.secondEntity.value)">
<span ng-bind="change.entity.secondEntity.value | limitTo: 250"></span>
<span
ng-bind="change.entity.secondEntity.value.length > 250 ? '...' :''"></span>
<span ng-bind="change.entity.secondEntity.value.length > 250 ? '...' :''"></span>
</td>
</tr>
</tbody>
Expand Down Expand Up @@ -310,7 +309,7 @@ <h4 class="text-center empty-container" ng-show="!isConfigHidden">
</div>
</section>

<showtextmodal text="text"></showtextmodal>
<showtextmodal text="text" new-str="newStr" old-str="oldStr" enable-text-diff="enableTextDiff"></showtextmodal>

<rollbackmodal app-id="pageContext.appId" env="pageContext.env" cluster="pageContext.clusterName">
</rollbackmodal>
Expand Down
1 change: 1 addition & 0 deletions apollo-portal/src/main/resources/static/i18n/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@
"Component.Publish.NoPublishedValue": "Unreleased values",
"Component.Publish.ModifyUser": "Modifier",
"Component.Publish.ModifyTime": "Modified Time",
"Component.Publish.ModifyRecord": "Record",
"Component.Publish.NewAdded": "New",
"Component.Publish.NewAddedTips": "New Configuration",
"Component.Publish.Modified": "Modified",
Expand Down
19 changes: 10 additions & 9 deletions apollo-portal/src/main/resources/static/i18n/zh-CN.json
Original file line number Diff line number Diff line change
Expand Up @@ -291,12 +291,13 @@
"Component.Publish.ToSeeChange": "查看变更",
"Component.Publish.CompareWithMasterValue": "与主版本对比",
"Component.Publish.CompareWithPublishedValue": "与已发布对比",
"Component.Publish.PublishedValue": "待发布的值",
"Component.Publish.Changes": "Changes",
"Component.Publish.PublishedValue": "已发布的值",
"Component.Publish.Changes": "改动",
"Component.Publish.Key": "Key",
"Component.Publish.NoPublishedValue": "未发布的值",
"Component.Publish.NoPublishedValue": "待发布的值",
"Component.Publish.ModifyUser": "修改人",
"Component.Publish.ModifyTime": "修改时间",
"Component.Publish.ModifyRecord": "修改记录",
"Component.Publish.NewAdded": "新",
"Component.Publish.NewAddedTips": "新增的配置",
"Component.Publish.Modified": "改",
Expand All @@ -310,8 +311,8 @@
"Component.Publish.ItemNoChange": "配置没有变化",
"Component.Publish.GrayItemNoChange": "灰度配置没有变化",
"Component.Publish.NoGrayItems": "没有灰度的配置项",
"Component.Publish.Release": "Release Name",
"Component.Publish.ReleaseComment": "Comment",
"Component.Publish.Release": "版本名称",
"Component.Publish.ReleaseComment": "说明",
"Component.Publish.OpPublish": "发布",
"Component.Rollback.To": "回滚到",
"Component.Rollback.Tips": "此操作将会回滚到上一个发布版本,且当前版本作废,但不影响正在修改的配置。可在发布历史页面查看当前生效的版本",
Expand Down Expand Up @@ -731,11 +732,11 @@
"Config.History.ChangedItemTips": "查看此次发布与上次版本的变更",
"Config.History.AllItem": "全部配置",
"Config.History.AllItemTips": "查看此次发布的所有配置信息",
"Config.History.ChangeType": "Type",
"Config.History.ChangeType": "类型",
"Config.History.ChangeKey": "Key",
"Config.History.ChangeValue": "Value",
"Config.History.ChangeOldValue": "Old Value",
"Config.History.ChangeNewValue": "New Value",
"Config.History.ChangeValue": "",
"Config.History.ChangeOldValue": "旧值",
"Config.History.ChangeNewValue": "新值",
"Config.History.ChangeTypeNew": "新增",
"Config.History.ChangeTypeModify": "修改",
"Config.History.ChangeTypeDelete": "删除",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ diff_item_module.controller("DiffItemController",
$scope.backToAppHomePage = backToAppHomePage;
$scope.switchSelect = switchSelect;

$scope.showText = showText;
$scope.showTextDiff = showTextDiff;

$scope.itemsKeyedByKey = {};
$scope.allNamespaceValueEqualed = {};
Expand Down Expand Up @@ -164,17 +164,19 @@ diff_item_module.controller("DiffItemController",
function parseSyncSourceData() {
var syncData = {
syncToNamespaces: [],
syncItems: []
syncItems: [],
firstClusterKey: "",
};
var namespaceName = $scope.pageContext.namespaceName;
selectedClusters.forEach(function (cluster) {
if (cluster.checked) {
cluster.clusterName = cluster.name;
cluster.namespaceName = namespaceName;
cluster.compositedKey = cluster.env + ':' + cluster.clusterName + ':' + cluster.namespaceName;
syncData.syncToNamespaces.push(cluster);
}
});

syncData.firstClusterKey = selectedClusters[0].compositedKey;
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add null check for selectedClusters array.

The code assumes selectedClusters[0] exists, which could lead to runtime errors if the array is empty.

-                syncData.firstClusterKey = selectedClusters[0].compositedKey;
+                syncData.firstClusterKey = selectedClusters.length > 0 ? selectedClusters[0].compositedKey : '';
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
syncData.firstClusterKey = selectedClusters[0].compositedKey;
syncData.firstClusterKey = selectedClusters.length > 0 ? selectedClusters[0].compositedKey : '';

return syncData;
}

Expand All @@ -193,8 +195,9 @@ diff_item_module.controller("DiffItemController",
o.checked = !o.checked;
}

function showText(text) {
$scope.text = text;
function showTextDiff(oldStr, newStr) {
$scope.oldStr = oldStr;
$scope.newStr = newStr;
AppUtil.showModal('#showTextModal');
}
}]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ function releaseHistoryController($scope, $location, $translate, AppUtil, EventM
$scope.switchConfigViewType = switchConfigViewType;
$scope.findReleaseHistory = findReleaseHistory;
$scope.showText = showText;
$scope.showTextDiff = showTextDiff;

EventManager.subscribe(EventManager.EventType.REFRESH_RELEASE_HISTORY, function () {
location.reload(true);
Expand Down Expand Up @@ -207,8 +208,16 @@ function releaseHistoryController($scope, $location, $translate, AppUtil, EventM
}

function showText(text) {
$scope.enableTextDiff = false;
$scope.text = text;
AppUtil.showModal("#showTextModal");
}

function showTextDiff(oldStr, newStr) {
$scope.enableTextDiff = true;
$scope.oldStr = oldStr;
$scope.newStr = newStr;
AppUtil.showModal('#showTextModal');
}
Comment on lines +216 to +221
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add input validation and error handling

The new text diff functionality needs additional safeguards for robustness:

  1. Input validation for oldStr and newStr
  2. Error handling for modal operations
  3. Cleanup of scope variables when modal is closed

Consider applying these improvements:

 function showTextDiff(oldStr, newStr) {
+    // Input validation
+    if (oldStr === undefined || newStr === undefined) {
+        AppUtil.showErrorMsg($translate.instant('Config.Text.InvalidInput'));
+        return;
+    }
+
     $scope.enableTextDiff = true;
     $scope.oldStr = oldStr;
     $scope.newStr = newStr;
-    AppUtil.showModal('#showTextModal');
+    
+    try {
+        AppUtil.showModal('#showTextModal');
+        
+        // Clean up when modal is closed
+        $('#showTextModal').on('hidden.bs.modal', function () {
+            $scope.$apply(function() {
+                $scope.enableTextDiff = false;
+                $scope.oldStr = null;
+                $scope.newStr = null;
+            });
+        });
+    } catch (error) {
+        AppUtil.showErrorMsg($translate.instant('Config.Text.ModalError'));
+    }
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function showTextDiff(oldStr, newStr) {
$scope.enableTextDiff = true;
$scope.oldStr = oldStr;
$scope.newStr = newStr;
AppUtil.showModal('#showTextModal');
}
function showTextDiff(oldStr, newStr) {
// Input validation
if (oldStr === undefined || newStr === undefined) {
AppUtil.showErrorMsg($translate.instant('Config.Text.InvalidInput'));
return;
}
$scope.enableTextDiff = true;
$scope.oldStr = oldStr;
$scope.newStr = newStr;
try {
AppUtil.showModal('#showTextModal');
// Clean up when modal is closed
$('#showTextModal').on('hidden.bs.modal', function () {
$scope.$apply(function() {
$scope.enableTextDiff = false;
$scope.oldStr = null;
$scope.newStr = null;
});
});
} catch (error) {
AppUtil.showErrorMsg($translate.instant('Config.Text.ModalError'));
}
}

}

Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ directive_module.directive('apollodiff',
part.removed ? 'red' : 'grey';
span = document.createElement('span');
span.style.color = color;
span.style.display = 'block';
pre = part.added ? '+' :
part.removed ? '-' : '';
span.appendChild(document.createTextNode(pre + part.value));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ function releaseModalDirective($translate, toastr, AppUtil, EventManager, Releas

scope.releaseBtnDisabled = false;
scope.releaseChangeViewType = 'compareWithPublishedValue';
scope.isComparePublished = true;
scope.releaseComment = '';
scope.isEmergencyPublish = false;

Expand Down Expand Up @@ -170,6 +171,9 @@ function releaseModalDirective($translate, toastr, AppUtil, EventManager, Releas

function switchReleaseChangeViewType(type) {
scope.releaseChangeViewType = type;
scope.isCompareMaster = type === 'compareWithMasterValue';
scope.isComparePublished = type === 'compareWithPublishedValue';
scope.isNoCompare = type === 'release';
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,10 @@ function showTextModalDirective(AppUtil) {
transclude: true,
replace: true,
scope: {
text: '='
text: '=',
oldStr: '=',
newStr: '=',
enableTextDiff: '='
},
link: function (scope) {
scope.$watch('text', init);
Expand Down
19 changes: 18 additions & 1 deletion apollo-portal/src/main/resources/static/styles/common-style.css
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,18 @@ p, td, span {
color: #797979;
}

pre, .pre {
pre {
white-space: pre-wrap; /* Since CSS 2.1 */
white-space: -moz-pre-wrap; /* Mozilla, since 1999 */
white-space: -pre-wrap; /* Opera 4-6 */
white-space: -o-pre-wrap; /* Opera 7 */
word-wrap: break-word; /* Internet Explorer 5.5+ */
}

.pre {
white-space: pre;
}

.hover:hover {
background: #f5f5f5;
cursor: pointer
Expand Down Expand Up @@ -1172,4 +1176,17 @@ table th {
#consumer-list .create-btn img {
width: 16px;
height: 16px;
}

.left-overflow {
overflow-x: auto;
text-align: left;
}

.table-fixed{
table-layout: fixed;
}

.block {
display: block;
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<pre class="no-radius" id="{{apolloId}}">
<pre class="no-radius pre" id="{{apolloId}}" style="margin:0">
<!--
~ Copyright 2024 Apollo Authors
~
Expand Down
Loading
Loading