add revoke item API. #2856 #2952

Accelerator96 · 2020-03-10T10:24:01Z

What's the purpose of this PR

add revoke item API.

Which issue(s) this PR fixes:
Fixes #2856 #2595

codecov-io · 2020-03-10T10:34:08Z

Codecov Report

Merging #2952 into master will increase coverage by 8.06%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #2952      +/-   ##
============================================
+ Coverage     51.33%   59.39%   +8.06%     
+ Complexity     2228     1240     -988     
============================================
  Files           432      203     -229     
  Lines         13335     6155    -7180     
  Branches       1368      654     -714     
============================================
- Hits           6846     3656    -3190     
+ Misses         6011     2226    -3785     
+ Partials        478      273     -205

Impacted Files	Coverage Δ	Complexity Δ
.../portal/spi/configuration/LdapGroupProperties.java
...lo/portal/controller/ReleaseHistoryController.java
...trip/framework/apollo/portal/enums/ChangeType.java
...mework/apollo/portal/controller/AppController.java
...trip/framework/apollo/portal/entity/vo/Number.java
...ponent/emailbuilder/ConfigPublishEmailBuilder.java
...nfigservice/ConfigServerEurekaServerConfigure.java
...mework/apollo/portal/service/NamespaceService.java
...mework/apollo/portal/entity/vo/EnvClusterInfo.java
...rk/apollo/metaservice/ApolloMetaServiceConfig.java
... and 219 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f3395f0...709ba8e. Read the comment docs.

coveralls · 2020-03-10T10:35:56Z

Coverage decreased (-0.2%) to 54.769% when pulling 709ba8e on Accelerater:#2856 into f3395f0 on ctripcorp:master.

nobodyiam

This is a nice try, however I think we need to combine the ui part to see how we could make this function work.

apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/ItemController.java

apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/ItemService.java

Accelerator96 · 2020-03-17T10:35:52Z

感谢 @bigflybrother 提供Portal页面支持。

nobodyiam

Basically it works nicely. However, there are some suggestions as below, please take a look.

BTW, would you please squash the commits so that one commit from @bigflybrother and one commit from @Accelerater? Because for each pr we would only record one commit per author.

nobodyiam · 2020-03-21T13:22:59Z

apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/ItemService.java

+    Env env = model.getEnv();
+    String clusterName = model.getClusterName();
+    String namespaceName = model.getNamespaceName();
+    long namespaceId = model.getNamespaceId();


please load the namespace and get the namespaceId instead of just using what's passed in since the namespaceId might not match the namespaceName

NamespaceDTO namespace = namespaceAPI.loadNamespace(appId, env, clusterName, namespaceName);

这里我是参考这个接口写的..
https://github.com/ctripcorp/apollo/blob/6d8686f66ee42440675c3e9447e27a1d33bf01e5/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/ItemController.java#L58

如果这样写会导致不匹配的话是不是上面这个接口也需要修改？

是的，modifyItemsByText这个也有潜在的安全风险

nobodyiam · 2020-03-21T13:23:50Z

apollo-portal/src/main/resources/static/i18n/zh-CN.json

@@ -345,6 +347,8 @@
  "Config.ClusterIsDefaultTipContent": "所有不属于 '{{name}}' 集群的实例会使用default集群（当前页面）的配置，属于 '{{name}}' 的实例会使用对应集群的配置！",
  "Config.ClusterIsCustomTipContent": "属于 '{{name}}' 集群的实例只会使用 '{{name}}' 集群（当前页面）的配置，只有当对应namespace在当前集群没有发布过配置时，才会使用default集群的配置。",
  "Config.HasNotPublishNamespace": "以下环境/集群有未发布的配置，客户端获取不到未发布的配置，请及时发布。",
+  "Config.RevokeItem.DialogTitle": "撤销配置",
+  "Config.RevokeItem.DialogContent": "当前命名空间下已修改但尚未未发布的配置将被撤销,确定要撤销么?",


当前命名空间下已修改但尚未发布的配置将被撤销，确定要撤销么?

这个中文提示有什么不对的地方么？

尚未未发布 => 尚未发布，撤销,确定 => 撤销，确定

我的眼瞎了😖

nobodyiam · 2020-03-21T13:24:23Z

apollo-portal/src/main/resources/static/scripts/controller/config/ConfigNamespaceController.js

@@ -178,6 +180,37 @@ function controller($rootScope, $scope, $translate, toastr, AppUtil, EventManage
                });
    }

+    function preRevokeItem(namespace) {
+        console.log("xxx")


please remove the debug message

nobodyiam · 2020-03-21T13:24:44Z

apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/ItemService.java

+      ItemDTO oldItem = oldKeyMapItem.get(key);
+      if (oldItem == null) {
+        ItemDTO deletedItemDto = deletedItemDTOs.computeIfAbsent(key, k -> new ItemDTO());
+        changeSets.addCreateItem(buildNormalItem(0L,namespaceId,key,value,deletedItemDto.getComment(),lineNum.get()));


the format doesn't look right here?

这里是为了获取到已删除配置的comment. 有哪里不对么..

code format is not right, e.g, 0L,namespaceId => 0L, namespaceId

nobodyiam · 2020-03-21T13:26:17Z

apollo-portal/src/main/resources/static/i18n/zh-CN.json

@@ -187,6 +187,8 @@
  "Component.Namespace.Master.Items.FilterItem": "过滤配置",
  "Component.Namespace.Master.Items.SyncItemTips": "同步各环境间配置",
  "Component.Namespace.Master.Items.SyncItem": "同步配置",
+  "Component.Namespace.Master.Items.RevokeItemTips": "撤销配置的修改",


please add the English translations to en.json

nobodyiam · 2020-03-22T08:43:13Z

apollo-portal/src/main/resources/static/i18n/en.json

@@ -345,6 +347,8 @@
  "Config.ClusterIsDefaultTipContent": "All instances that do not belong to the '{{name}}' cluster will fetch the default cluster (current page) configuration, and those that belong to the '{{name}}' cluster will use the corresponding cluster configuration!",
  "Config.ClusterIsCustomTipContent": "Instances belonging to the '{{name}}' cluster will only fetch the configuration of the '{{name}}' cluster (the current page), and the default cluster configuration will only be fetched when the corresponding namespace has not been released in the current cluster.",
  "Config.HasNotPublishNamespace": "The following environment/cluster has unreleased configurations, the client will not fetch the unreleased configurations, please release them in time.",
+  "Config.RevokeItem.DialogTitle": "Revoke configuration",
+  "Config.RevokeItem.DialogContent": "Modified but unpublished configurations in the current namespace will be revoked. Are you sure to revoke the configuration?",


I think Revoke.RevokeFailed and Revoke.RevokeSuccessfully are still missing?

nobodyiam · 2020-03-22T08:43:55Z

apollo-portal/src/main/resources/static/i18n/en.json

@@ -185,6 +185,8 @@
  "Component.Namespace.Master.Items.SummitChanged": "Submit",
  "Component.Namespace.Master.Items.SortByKey": "Filter the configurations by key",
  "Component.Namespace.Master.Items.FilterItem": "Filter",
+  "Component.Namespace.Master.Items.RevokeItemTips": "Revoke configurations",


How about Revoke configuration changes?

nobodyiam · 2020-03-22T08:45:18Z

apollo-portal/src/main/resources/static/i18n/en.json

@@ -345,6 +347,8 @@
  "Config.ClusterIsDefaultTipContent": "All instances that do not belong to the '{{name}}' cluster will fetch the default cluster (current page) configuration, and those that belong to the '{{name}}' cluster will use the corresponding cluster configuration!",
  "Config.ClusterIsCustomTipContent": "Instances belonging to the '{{name}}' cluster will only fetch the configuration of the '{{name}}' cluster (the current page), and the default cluster configuration will only be fetched when the corresponding namespace has not been released in the current cluster.",
  "Config.HasNotPublishNamespace": "The following environment/cluster has unreleased configurations, the client will not fetch the unreleased configurations, please release them in time.",
+  "Config.RevokeItem.DialogTitle": "Revoke configuration",


How about Revoke configuration changes?

nobodyiam · 2020-03-22T08:47:29Z

apollo-portal/src/main/resources/static/i18n/en.json

@@ -345,6 +347,8 @@
  "Config.ClusterIsDefaultTipContent": "All instances that do not belong to the '{{name}}' cluster will fetch the default cluster (current page) configuration, and those that belong to the '{{name}}' cluster will use the corresponding cluster configuration!",
  "Config.ClusterIsCustomTipContent": "Instances belonging to the '{{name}}' cluster will only fetch the configuration of the '{{name}}' cluster (the current page), and the default cluster configuration will only be fetched when the corresponding namespace has not been released in the current cluster.",
  "Config.HasNotPublishNamespace": "The following environment/cluster has unreleased configurations, the client will not fetch the unreleased configurations, please release them in time.",
+  "Config.RevokeItem.DialogTitle": "Revoke configuration",
+  "Config.RevokeItem.DialogContent": "Modified but unpublished configurations in the current namespace will be revoked. Are you sure to revoke the configuration?",


How about Modified but unpublished configurations in the current namespace will be revoked. Are you sure to revoke the configuration changes?

nobodyiam

LGTM

add revoke item API.

0739a62

nobodyiam requested changes Mar 14, 2020

View reviewed changes

apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/ItemController.java Show resolved Hide resolved

apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/ItemService.java Outdated Show resolved Hide resolved

Merge branch 'master' into #2856

b5443e2

JaredTan95 added enhancement feature Categorizes issue as related to a new feature. labels Mar 17, 2020

JaredTan95 added this to the 1.7.0 milestone Mar 17, 2020

nobodyiam requested changes Mar 21, 2020

View reviewed changes

zhuohao.li and others added 2 commits March 22, 2020 14:45

fix and add ui.

9132d42

Merge branch 'master' into #2856

cbde22a

nobodyiam reviewed Mar 22, 2020

View reviewed changes

Modify English translation.

709ba8e

nobodyiam approved these changes Mar 22, 2020

View reviewed changes

nobodyiam merged commit 9f4bac4 into apolloconfig:master Mar 22, 2020

Accelerator96 deleted the #2856 branch March 22, 2020 09:33

This was referenced Mar 29, 2020

回滚操作问题,版本1.4 #2458

Closed

有一次发布历史，点击“回滚”后显示“没有可以回滚的发布历史” #2595

Closed

nobodyiam mentioned this pull request Jun 13, 2020

能否提供发布之前中间状态的便捷撤销功能？ #3117

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add revoke item API. #2856 #2952

add revoke item API. #2856 #2952

Accelerator96 commented Mar 10, 2020

codecov-io commented Mar 10, 2020 •

edited

Loading

coveralls commented Mar 10, 2020 •

edited

Loading

nobodyiam left a comment

Accelerator96 commented Mar 17, 2020

nobodyiam left a comment •

edited

Loading

nobodyiam Mar 21, 2020

Accelerator96 Mar 21, 2020

nobodyiam Mar 22, 2020

nobodyiam Mar 21, 2020

Accelerator96 Mar 21, 2020

nobodyiam Mar 22, 2020

Accelerator96 Mar 22, 2020

nobodyiam Mar 21, 2020

nobodyiam Mar 21, 2020

Accelerator96 Mar 21, 2020

nobodyiam Mar 22, 2020

Accelerator96 Mar 22, 2020

nobodyiam Mar 21, 2020

Accelerator96 Mar 21, 2020

nobodyiam Mar 22, 2020

nobodyiam Mar 22, 2020 •

edited

Loading

nobodyiam Mar 22, 2020

nobodyiam Mar 22, 2020

nobodyiam left a comment

add revoke item API. #2856 #2952

add revoke item API. #2856 #2952

Conversation

Accelerator96 commented Mar 10, 2020

codecov-io commented Mar 10, 2020 • edited Loading

Codecov Report

coveralls commented Mar 10, 2020 • edited Loading

nobodyiam left a comment

Choose a reason for hiding this comment

Accelerator96 commented Mar 17, 2020

nobodyiam left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nobodyiam Mar 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nobodyiam left a comment

Choose a reason for hiding this comment

codecov-io commented Mar 10, 2020 •

edited

Loading

coveralls commented Mar 10, 2020 •

edited

Loading

nobodyiam left a comment •

edited

Loading

nobodyiam Mar 22, 2020 •

edited

Loading