You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2022/05/24 16:10:14 UTC

[GitHub] [trafficcontrol] rimashah25 opened a new pull request, #6855: Updated success message (curl and TP) for deleting a DS

rimashah25 opened a new pull request, #6855:
URL: https://github.com/apache/trafficcontrol/pull/6855

   <!--
   Thank you for contributing! Please be sure to read our contribution guidelines: https://github.com/apache/trafficcontrol/blob/master/CONTRIBUTING.md
   If this closes or relates to an existing issue, please reference it using one of the following:
   
   Closes: #ISSUE
   Related: #ISSUE
   
   If this PR fixes a security vulnerability, DO NOT submit! Instead, contact
   the Apache Traffic Control Security Team at security@trafficcontrol.apache.org and follow the
   guidelines at https://apache.org/security regarding vulnerability disclosure.
   -->
   Related: #4351 
   
   
   <!-- **^ Add meaningful description above** --><hr/>
   
   ## Which Traffic Control components are affected by this PR?
   <!-- Please delete all components from this list that are NOT affected by this PR.
   Feel free to add the name of a tool or script that is affected but not on the list.
   -->
   - Traffic Ops
   - Traffic Portal
   
   ## What is the best way to verify this PR?
   <!-- Please include here ALL the steps necessary to test your PR.
   If your PR has tests (and most should), provide the steps needed to run the tests.
   If not, please provide step-by-step instructions to test the PR manually and explain why your PR does not need tests. -->
   Create a DS or region or server
   Delete a DS (see updated message), region and server (have same message as before i.e. `server/region was deleted.` on TO and `server/region deleted` on TP.
   
   Updated message for DS
   ```
   curl -X DELETE -k -H "Accept: application/json" --cookie "mojolicious=<mojolicious cookie>" https://localhost:8443/api/4.0/deliveryservices/<ds id>
   
   {"alerts":[{
          "text":"ds was deleted. Perform a CDN snapshot then queue updates on all servers in the cdn for the changes to take affect.",
          "level":"success"
   }]
   ```
   
   ## If this is a bugfix, which Traffic Control versions contained the bug?
   <!-- Delete this section if the PR is not a bugfix, or if the bug is only in the master branch.
   Examples:
   - 5.1.2
   - 5.1.3 (RC1)
    -->
   
   
   ## PR submission checklist
   - [ ] This PR has tests <!-- If not, please delete this text and explain why this PR does not need tests. -->
   - [ ] This PR has documentation <!-- If not, please delete this text and explain why this PR does not need documentation. -->
   - [x] This PR has a CHANGELOG.md entry <!-- A fix for a bug from an ATC release, an improvement, or a new feature should have a changelog entry. -->
   - [x] This PR **DOES NOT FIX A SERIOUS SECURITY VULNERABILITY** (see [the Apache Software Foundation's security guidelines](https://apache.org/security) for details)
   
   <!--
   Licensed to the Apache Software Foundation (ASF) under one
   or more contributor license agreements.  See the NOTICE file
   distributed with this work for additional information
   regarding copyright ownership.  The ASF licenses this file
   to you under the Apache License, Version 2.0 (the
   "License"); you may not use this file except in compliance
   with the License.  You may obtain a copy of the License at
   
       http://www.apache.org/licenses/LICENSE-2.0
   
   Unless required by applicable law or agreed to in writing,
   software distributed under the License is distributed on an
   "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
   KIND, either express or implied.  See the License for the
   specific language governing permissions and limitations
   under the License.
   -->
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #6855: Updated success message (curl and TP) for deleting a DS

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on code in PR #6855:
URL: https://github.com/apache/trafficcontrol/pull/6855#discussion_r881999104


##########
traffic_portal/app/src/common/modules/form/deliveryService/edit/FormEditDeliveryServiceController.js:
##########
@@ -299,7 +299,10 @@ var FormEditDeliveryServiceController = function(deliveryService, origin, topolo
 				deliveryServiceService.deleteDeliveryService(deliveryService)
 					.then(
 						function() {
-							messageModel.setMessages([ { level: 'success', text: 'Delivery service [ ' + deliveryService.xmlId + ' ] deleted' } ], true);
+							messageModel.setMessages([

Review Comment:
   two main reasons
   
   1. That's what it already was doing, so this PR merely isn't a fix for it - it's not introducing the unwanted behavior.
   2. This hard-codes the response, because all Traffic Ops returns is "ds was deleted". The custom Alert puts the XMLID in the message. Fixing Traffic Ops to include that information is involved, because Delivery Services use the CRUDer, so it's difficult to change nearly anything about the endpoint without retooling the whole API. Which is out of scope for this change.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] rimashah25 commented on a diff in pull request #6855: Updated success message (curl and TP) for deleting a DS

Posted by GitBox <gi...@apache.org>.
rimashah25 commented on code in PR #6855:
URL: https://github.com/apache/trafficcontrol/pull/6855#discussion_r882121021


##########
traffic_portal/app/src/common/modules/form/deliveryService/edit/FormEditDeliveryServiceController.js:
##########
@@ -299,7 +299,10 @@ var FormEditDeliveryServiceController = function(deliveryService, origin, topolo
 				deliveryServiceService.deleteDeliveryService(deliveryService)
 					.then(
 						function() {
-							messageModel.setMessages([ { level: 'success', text: 'Delivery service [ ' + deliveryService.xmlId + ' ] deleted' } ], true);
+							messageModel.setMessages([

Review Comment:
   @ocket8888 @mitchell852 I am creating another to remove hardcoding and updating tests to use the updated validationMessage.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] mitchell852 commented on a diff in pull request #6855: Updated success message (curl and TP) for deleting a DS

Posted by GitBox <gi...@apache.org>.
mitchell852 commented on code in PR #6855:
URL: https://github.com/apache/trafficcontrol/pull/6855#discussion_r881983004


##########
traffic_portal/app/src/common/modules/form/deliveryService/edit/FormEditDeliveryServiceController.js:
##########
@@ -299,7 +299,10 @@ var FormEditDeliveryServiceController = function(deliveryService, origin, topolo
 				deliveryServiceService.deleteDeliveryService(deliveryService)
 					.then(
 						function() {
-							messageModel.setMessages([ { level: 'success', text: 'Delivery service [ ' + deliveryService.xmlId + ' ] deleted' } ], true);
+							messageModel.setMessages([

Review Comment:
   why? TP can just show the message(s) sent from the API with messageModel.setMessages(result.alerts, true) or something like that. This approach has essentially hard-coded the api message into the UI which is problematic.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] mitchell852 commented on a diff in pull request #6855: Updated success message (curl and TP) for deleting a DS

Posted by GitBox <gi...@apache.org>.
mitchell852 commented on code in PR #6855:
URL: https://github.com/apache/trafficcontrol/pull/6855#discussion_r881983004


##########
traffic_portal/app/src/common/modules/form/deliveryService/edit/FormEditDeliveryServiceController.js:
##########
@@ -299,7 +299,10 @@ var FormEditDeliveryServiceController = function(deliveryService, origin, topolo
 				deliveryServiceService.deleteDeliveryService(deliveryService)
 					.then(
 						function() {
-							messageModel.setMessages([ { level: 'success', text: 'Delivery service [ ' + deliveryService.xmlId + ' ] deleted' } ], true);
+							messageModel.setMessages([

Review Comment:
   why? TP can just show the message(s) sent from the API with messageModel.setMessages(result.alerts) or something like that. This approach has essentially hard-coded the api message into the UI.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] rimashah25 commented on a diff in pull request #6855: Updated success message (curl and TP) for deleting a DS

Posted by GitBox <gi...@apache.org>.
rimashah25 commented on code in PR #6855:
URL: https://github.com/apache/trafficcontrol/pull/6855#discussion_r881947977


##########
traffic_portal/test/integration/Data/deliveryservices.ts:
##########
@@ -357,17 +357,17 @@ export const deliveryservices = {
 				{
 					description: "delete a delivery service",
 					Name: "tpdservice1",
-					validationMessage: "Delivery service [ tpdservice1 ] deleted"
+					validationMessage: "Delivery service [ tpdservice1 ] deleted."

Review Comment:
   I think in this case a period makes sense since there are two messages, even though they are as two separate level.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #6855: Updated success message (curl and TP) for deleting a DS

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on code in PR #6855:
URL: https://github.com/apache/trafficcontrol/pull/6855#discussion_r882082426


##########
traffic_portal/app/src/common/modules/form/deliveryService/edit/FormEditDeliveryServiceController.js:
##########
@@ -299,7 +299,10 @@ var FormEditDeliveryServiceController = function(deliveryService, origin, topolo
 				deliveryServiceService.deleteDeliveryService(deliveryService)
 					.then(
 						function() {
-							messageModel.setMessages([ { level: 'success', text: 'Delivery service [ ' + deliveryService.xmlId + ' ] deleted' } ], true);
+							messageModel.setMessages([

Review Comment:
   I'm not sure either. It should be changed, but that can be done in a separate PR.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] rimashah25 commented on a diff in pull request #6855: Updated success message (curl and TP) for deleting a DS

Posted by GitBox <gi...@apache.org>.
rimashah25 commented on code in PR #6855:
URL: https://github.com/apache/trafficcontrol/pull/6855#discussion_r882121021


##########
traffic_portal/app/src/common/modules/form/deliveryService/edit/FormEditDeliveryServiceController.js:
##########
@@ -299,7 +299,10 @@ var FormEditDeliveryServiceController = function(deliveryService, origin, topolo
 				deliveryServiceService.deleteDeliveryService(deliveryService)
 					.then(
 						function() {
-							messageModel.setMessages([ { level: 'success', text: 'Delivery service [ ' + deliveryService.xmlId + ' ] deleted' } ], true);
+							messageModel.setMessages([

Review Comment:
   @ocket8888 @mitchell852 I am creating another to remove hardcoding and updating test to use the updated validationMessage.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] mitchell852 commented on a diff in pull request #6855: Updated success message (curl and TP) for deleting a DS

Posted by GitBox <gi...@apache.org>.
mitchell852 commented on code in PR #6855:
URL: https://github.com/apache/trafficcontrol/pull/6855#discussion_r882050319


##########
traffic_portal/app/src/common/modules/form/deliveryService/edit/FormEditDeliveryServiceController.js:
##########
@@ -299,7 +299,10 @@ var FormEditDeliveryServiceController = function(deliveryService, origin, topolo
 				deliveryServiceService.deleteDeliveryService(deliveryService)
 					.then(
 						function() {
-							messageModel.setMessages([ { level: 'success', text: 'Delivery service [ ' + deliveryService.xmlId + ' ] deleted' } ], true);
+							messageModel.setMessages([

Review Comment:
   i'll defer to you guys. in my experience hard coding is not a good idea and not sure why it was hardcoded in the first place. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #6855: Updated success message (curl and TP) for deleting a DS

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on code in PR #6855:
URL: https://github.com/apache/trafficcontrol/pull/6855#discussion_r881923038


##########
traffic_portal/test/integration/Data/deliveryservices.ts:
##########
@@ -357,17 +357,17 @@ export const deliveryservices = {
 				{
 					description: "delete a delivery service",
 					Name: "tpdservice1",
-					validationMessage: "Delivery service [ tpdservice1 ] deleted"
+					validationMessage: "Delivery service [ tpdservice1 ] deleted."

Review Comment:
   Do you think it's better with the period? I think a fair few of the Alerts we send from the API have punctuation, but also a lot don't. To be fair, the actual Alert TO sends has one.



##########
traffic_ops/traffic_ops_golang/api/shared_handlers.go:
##########
@@ -324,7 +324,13 @@ func DeleteHandler(deleter Deleter) http.HandlerFunc {
 		deleter,
 		HandleErr,
 		func(w http.ResponseWriter, r *http.Request, message string) {
-			WriteRespAlert(w, r, tc.SuccessLevel, message)
+			if deleter.GetType() == "ds" {
+				alerts := tc.CreateAlerts(tc.SuccessLevel, message)
+				alerts.AddNewAlert(tc.InfoLevel, "Perform a CDN snapshot then queue updates on all servers in the cdn for the changes to take affect.")

Review Comment:
   grammar: "affect" should be "effect"



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] mitchell852 commented on a diff in pull request #6855: Updated success message (curl and TP) for deleting a DS

Posted by GitBox <gi...@apache.org>.
mitchell852 commented on code in PR #6855:
URL: https://github.com/apache/trafficcontrol/pull/6855#discussion_r881983004


##########
traffic_portal/app/src/common/modules/form/deliveryService/edit/FormEditDeliveryServiceController.js:
##########
@@ -299,7 +299,10 @@ var FormEditDeliveryServiceController = function(deliveryService, origin, topolo
 				deliveryServiceService.deleteDeliveryService(deliveryService)
 					.then(
 						function() {
-							messageModel.setMessages([ { level: 'success', text: 'Delivery service [ ' + deliveryService.xmlId + ' ] deleted' } ], true);
+							messageModel.setMessages([

Review Comment:
   why? TP can just show the message(s) sent from the API. This approach has essentially hard-coded the api message.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] rimashah25 commented on a diff in pull request #6855: Updated success message (curl and TP) for deleting a DS

Posted by GitBox <gi...@apache.org>.
rimashah25 commented on code in PR #6855:
URL: https://github.com/apache/trafficcontrol/pull/6855#discussion_r882121021


##########
traffic_portal/app/src/common/modules/form/deliveryService/edit/FormEditDeliveryServiceController.js:
##########
@@ -299,7 +299,10 @@ var FormEditDeliveryServiceController = function(deliveryService, origin, topolo
 				deliveryServiceService.deleteDeliveryService(deliveryService)
 					.then(
 						function() {
-							messageModel.setMessages([ { level: 'success', text: 'Delivery service [ ' + deliveryService.xmlId + ' ] deleted' } ], true);
+							messageModel.setMessages([

Review Comment:
   @ocket8888 @mitchell852 I am creating another PR 6860(https://github.com/apache/trafficcontrol/pull/6860) to remove hardcoding (DS create, update and delete) and updating associated tests to use latest validationMessage.



##########
traffic_portal/app/src/common/modules/form/deliveryService/edit/FormEditDeliveryServiceController.js:
##########
@@ -299,7 +299,10 @@ var FormEditDeliveryServiceController = function(deliveryService, origin, topolo
 				deliveryServiceService.deleteDeliveryService(deliveryService)
 					.then(
 						function() {
-							messageModel.setMessages([ { level: 'success', text: 'Delivery service [ ' + deliveryService.xmlId + ' ] deleted' } ], true);
+							messageModel.setMessages([

Review Comment:
   @ocket8888 @mitchell852 I am creating another PR(https://github.com/apache/trafficcontrol/pull/6860) to remove hardcoding (DS create, update and delete) and updating associated tests to use latest validationMessage.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] mitchell852 commented on a diff in pull request #6855: Updated success message (curl and TP) for deleting a DS

Posted by GitBox <gi...@apache.org>.
mitchell852 commented on code in PR #6855:
URL: https://github.com/apache/trafficcontrol/pull/6855#discussion_r882047352


##########
traffic_portal/app/src/common/modules/form/deliveryService/edit/FormEditDeliveryServiceController.js:
##########
@@ -299,7 +299,10 @@ var FormEditDeliveryServiceController = function(deliveryService, origin, topolo
 				deliveryServiceService.deleteDeliveryService(deliveryService)
 					.then(
 						function() {
-							messageModel.setMessages([ { level: 'success', text: 'Delivery service [ ' + deliveryService.xmlId + ' ] deleted' } ], true);
+							messageModel.setMessages([

Review Comment:
   but i see
   
   `				alerts := tc.CreateAlerts(tc.SuccessLevel, message)
   				alerts.AddNewAlert(tc.InfoLevel, "Perform a CDN snapshot then queue updates on all servers in the cdn for the changes to take effect.")
   `
   
   why not just show those. i think it's fine that the xmlId is lost in the UI.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] ocket8888 merged pull request #6855: Updated success message (curl and TP) for deleting a DS

Posted by GitBox <gi...@apache.org>.
ocket8888 merged PR #6855:
URL: https://github.com/apache/trafficcontrol/pull/6855


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] rimashah25 commented on a diff in pull request #6855: Updated success message (curl and TP) for deleting a DS

Posted by GitBox <gi...@apache.org>.
rimashah25 commented on code in PR #6855:
URL: https://github.com/apache/trafficcontrol/pull/6855#discussion_r882028788


##########
traffic_portal/app/src/common/modules/form/deliveryService/edit/FormEditDeliveryServiceController.js:
##########
@@ -299,7 +299,10 @@ var FormEditDeliveryServiceController = function(deliveryService, origin, topolo
 				deliveryServiceService.deleteDeliveryService(deliveryService)
 					.then(
 						function() {
-							messageModel.setMessages([ { level: 'success', text: 'Delivery service [ ' + deliveryService.xmlId + ' ] deleted' } ], true);
+							messageModel.setMessages([

Review Comment:
   Exactly what Brennan said. He had the same question and I explained why it is difficult to change things on TP since cruder on TO gives a generic message, hence developers of TP set a different message than results.alerts.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] ocket8888 commented on pull request #6855: Updated success message (curl and TP) for deleting a DS

Posted by GitBox <gi...@apache.org>.
ocket8888 commented on PR #6855:
URL: https://github.com/apache/trafficcontrol/pull/6855#issuecomment-1137468644

   Should this new message maybe go in an `info`-level alert instead of appending it to the success message?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] mitchell852 commented on a diff in pull request #6855: Updated success message (curl and TP) for deleting a DS

Posted by GitBox <gi...@apache.org>.
mitchell852 commented on code in PR #6855:
URL: https://github.com/apache/trafficcontrol/pull/6855#discussion_r881983004


##########
traffic_portal/app/src/common/modules/form/deliveryService/edit/FormEditDeliveryServiceController.js:
##########
@@ -299,7 +299,10 @@ var FormEditDeliveryServiceController = function(deliveryService, origin, topolo
 				deliveryServiceService.deleteDeliveryService(deliveryService)
 					.then(
 						function() {
-							messageModel.setMessages([ { level: 'success', text: 'Delivery service [ ' + deliveryService.xmlId + ' ] deleted' } ], true);
+							messageModel.setMessages([

Review Comment:
   why? TP can just show the message(s) sent from the API with messageModel.setMessages(result.alerts, true) or something like that. This approach has essentially hard-coded the api message into the UI.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] mitchell852 commented on a diff in pull request #6855: Updated success message (curl and TP) for deleting a DS

Posted by GitBox <gi...@apache.org>.
mitchell852 commented on code in PR #6855:
URL: https://github.com/apache/trafficcontrol/pull/6855#discussion_r881983004


##########
traffic_portal/app/src/common/modules/form/deliveryService/edit/FormEditDeliveryServiceController.js:
##########
@@ -299,7 +299,10 @@ var FormEditDeliveryServiceController = function(deliveryService, origin, topolo
 				deliveryServiceService.deleteDeliveryService(deliveryService)
 					.then(
 						function() {
-							messageModel.setMessages([ { level: 'success', text: 'Delivery service [ ' + deliveryService.xmlId + ' ] deleted' } ], true);
+							messageModel.setMessages([

Review Comment:
   why? TP can just show the message(s) sent from the API by messageModel.setMessages(result.alerts) or something like that. This approach has essentially hard-coded the api message into the UI.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [trafficcontrol] rimashah25 commented on a diff in pull request #6855: Updated success message (curl and TP) for deleting a DS

Posted by GitBox <gi...@apache.org>.
rimashah25 commented on code in PR #6855:
URL: https://github.com/apache/trafficcontrol/pull/6855#discussion_r882121021


##########
traffic_portal/app/src/common/modules/form/deliveryService/edit/FormEditDeliveryServiceController.js:
##########
@@ -299,7 +299,10 @@ var FormEditDeliveryServiceController = function(deliveryService, origin, topolo
 				deliveryServiceService.deleteDeliveryService(deliveryService)
 					.then(
 						function() {
-							messageModel.setMessages([ { level: 'success', text: 'Delivery service [ ' + deliveryService.xmlId + ' ] deleted' } ], true);
+							messageModel.setMessages([

Review Comment:
   @ocket8888 @mitchell852 I am creating another PR to remove hardcoding (DS create, update and delete) and updating associated tests to use latest validationMessage.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org