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 2021/11/05 21:27:14 UTC
[GitHub] [trafficcontrol] mattjackson220 opened a new pull request #6339: Cert expirations tp
mattjackson220 opened a new pull request #6339:
URL: https://github.com/apache/trafficcontrol/pull/6339
<!--
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.
-->
Closes: #2526
Requires PR #6337 and PR #6338 to be merged first!
This PR creates a page in Traffic Portal with a table of all SSL certificate expirations.
<!-- **^ 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 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. -->
Run Traffic Portal and verify that the Certificate Expirations button is available in the navigation bar on the left side under Services
Verify that the page loads successfully and shows the correct SSL cert expiration data
Verify that column filtering and searching works
Verify that the `Days` search works
## 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
- [x] This PR has tests <!-- If not, please delete this text and explain why this PR does not need tests. -->
- [x] 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] mattjackson220 commented on a change in pull request #6339: Cert expirations tp
Posted by GitBox <gi...@apache.org>.
mattjackson220 commented on a change in pull request #6339:
URL: https://github.com/apache/trafficcontrol/pull/6339#discussion_r781611821
##########
File path: traffic_portal/app/src/common/modules/table/certExpirations/TableCertExpirationsController.js
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.
+ */
+
+var TableCertExpirationsController = function(tableName, certExpirations, dsXmlToIdMap, filter, $document, $scope, $state, $filter, locationUtils) {
+
+ /** All of the expiration fields converted to actual Dates */
+ $scope.certExpirations = certExpirations.map(
+ function(x) {
+ // need to convert this to a date object for ag-grid filter to work properly
+ x.expiration = new Date(x.expiration);
+ return x;
+ });
+
+ $scope.dsXmlToIdMap = dsXmlToIdMap;
+
+ $scope.editCertExpirations = function(dsId) {
+ locationUtils.navigateToPath('/delivery-services/' + dsId + '/ssl-keys');
+ }
+
+ /**
+ * Formats the contents of a 'federation' column cell as "True" or blank for visibility.
+ */
+ function federatedCellFormatter(params) {
+ if (!params.value) {
+ return '';
+ } else {
+ return params.value;
+ }
Review comment:
yea i put it as 'true' or blank because when it was 'true' or 'false' it was not super clear at just a glance. so this way its easy to see quickly
--
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 change in pull request #6339: Cert expirations tp
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #6339:
URL: https://github.com/apache/trafficcontrol/pull/6339#discussion_r779048385
##########
File path: traffic_portal/app/src/common/modules/table/certExpirations/TableCertExpirationsController.js
##########
@@ -17,27 +17,24 @@
* under the License.
*/
-var TableCertExpirationsController = function(tableName, certExpirations, dsXmlToIdMap, filter, $document, $scope, $state, $filter, locationUtils, certExpirationsService) {
-
- let table;
+var TableCertExpirationsController = function(tableName, certExpirations, dsXmlToIdMap, filter, $document, $scope, $state, $filter, locationUtils) {
+
+ /** All of the expiration fields converted to actual Dates */
+ $scope.certExpirations = certExpirations.map(
+ function(x) {
+ // need to convert this to a date object for ag-grid filter to work properly
+ x.expiration = new Date(x.expiration.replace("+00", "Z"));
Review comment:
The timestamps returned by the API are RFC3339 with nanosecond precision, so this replacement is unnecessary.
--
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] mattjackson220 commented on a change in pull request #6339: Cert expirations tp
Posted by GitBox <gi...@apache.org>.
mattjackson220 commented on a change in pull request #6339:
URL: https://github.com/apache/trafficcontrol/pull/6339#discussion_r778925403
##########
File path: traffic_portal/app/src/common/modules/table/certExpirations/table.certExpirations.tpl.html
##########
@@ -0,0 +1,70 @@
+<!--
+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.
+-->
+
+<div class="x_panel">
+ <div class="x_title">
+ <ol class="breadcrumb pull-left">
+ <li class="active">Certificate Expirations</li>
+ </ol>
+ <div class="pull-right">
+ <div class="form-inline" role="search">
+ <input id="quickSearch" name="quickSearch" type="search" class="form-control text-input" placeholder="Quick search..." ng-model="quickSearch" ng-change="onQuickSearchChanged()" aria-label="Search"/>
+ <div class="input-group text-input">
+ <span class="input-group-addon">
+ <label for="days">Days</label>
+ </span>
+ <input id="days" name="days" type="number" min="1" class="form-control" placeholder="" ng-model="days" ng-change="onDaysChanged(days)" aria-label="Days"/>
Review comment:
i removed the `days` aspect completely and used the common ag grid instead
--
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 change in pull request #6339: Cert expirations tp
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #6339:
URL: https://github.com/apache/trafficcontrol/pull/6339#discussion_r781653372
##########
File path: traffic_portal/test/integration/PageObjects/SideNavigationPage.po.ts
##########
@@ -158,10 +158,9 @@ export class SideNavigationPage extends BasePage{
await browser.actions().mouseMove(this.lnkServiceCategories).perform();
await browser.actions().click(this.lnkServiceCategories).perform();
}
- async NavigateToCertExpirationsPage(){
- await browser.wait(ExpectedConditions.visibilityOf(this.lnkCertExpirations), 2000);
- await browser.actions().mouseMove(this.lnkCertExpirations).perform();
- await browser.actions().click(this.lnkCertExpirations).perform();
+ // NavigateToCertExpirationsPage verifies that the link to the Certificate Expirations page is clickable.
Review comment:
JSDoc comments need to be started with `/**` (and therefore ended with `*/`) in order to meet the spec. IDEs and the `jsdoc` program that they use won't pick it up otherwise.
##########
File path: traffic_portal/app/src/modules/private/certExpirations/list/index.js
##########
@@ -32,19 +32,6 @@ module.exports = angular.module('trafficPortal.private.certExpirations.list', []
},
tableName: function() {
return 'cert-expirations';
- },
- filter: function() {
- return null;
- },
- deliveryservices: function(deliveryServiceService) {
- return deliveryServiceService.getDeliveryServices();
- },
Review comment:
this is most of what `resolve` values are for: you get a promise that asynchronously outputs data, and Angular handles resolving it to an actual value injected into your controller at component initialization time. If you keep `deliveryservices` here, you can input it as an injected dependency like it used to be, and instead of a function like
```javascript
function init() {
doSomething();
}
init();
```
(which, incidentally is in no way any different than just calling `doSomething` immediately - the only real differences are `this` binding and the values of some global variables that no one should ever use like `name` and `arguments`)
you can just directly construct the map immediately like:
```javascript
$scope.dsXmlToIdMap = new Map(deliveryservices.map(ds=>[ds.xmlId, ds.id]));
```
no extra function wrapping or deferred calling or `.then`-ing a promise necessary at all.
--
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] shamrickus commented on a change in pull request #6339: Cert expirations tp
Posted by GitBox <gi...@apache.org>.
shamrickus commented on a change in pull request #6339:
URL: https://github.com/apache/trafficcontrol/pull/6339#discussion_r781537280
##########
File path: traffic_portal/app/src/common/modules/table/certExpirations/table.certExpirations.tpl.html
##########
@@ -0,0 +1,23 @@
+<!--
+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.
+-->
+
+<div class="x_panel">
+ <common-grid-controller title="Certificate Expirations" table-name="certExpirations" options="gridOptions" data="certExpirations"
Review comment:
Just merged [6415](https://github.com/apache/trafficcontrol/pull/6415), `title` needs to be `table-title` now.
--
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 change in pull request #6339: Cert expirations tp
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #6339:
URL: https://github.com/apache/trafficcontrol/pull/6339#discussion_r781624632
##########
File path: traffic_portal/app/src/common/modules/table/certExpirations/TableCertExpirationsController.js
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.
+ */
+
+var TableCertExpirationsController = function(tableName, certExpirations, dsXmlToIdMap, filter, $document, $scope, $state, $filter, locationUtils) {
+
+ /** All of the expiration fields converted to actual Dates */
+ $scope.certExpirations = certExpirations.map(
+ function(x) {
+ // need to convert this to a date object for ag-grid filter to work properly
+ x.expiration = new Date(x.expiration);
+ return x;
+ });
+
+ $scope.dsXmlToIdMap = dsXmlToIdMap;
+
+ $scope.editCertExpirations = function(dsId) {
+ locationUtils.navigateToPath('/delivery-services/' + dsId + '/ssl-keys');
+ }
+
+ /**
+ * Formats the contents of a 'federation' column cell as "True" or blank for visibility.
+ */
+ function federatedCellFormatter(params) {
+ if (!params.value) {
+ return '';
+ } else {
+ return params.value;
+ }
Review comment:
Okay, that's fine with me. You could also change it to a checkmark or something if you like that better - I think the cache checks page has an example of that - but as far as I'm concerned this is fine.
--
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 #6339: Cert expirations tp
Posted by GitBox <gi...@apache.org>.
ocket8888 merged pull request #6339:
URL: https://github.com/apache/trafficcontrol/pull/6339
--
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 change in pull request #6339: Cert expirations tp
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #6339:
URL: https://github.com/apache/trafficcontrol/pull/6339#discussion_r781526634
##########
File path: traffic_portal/app/src/common/modules/table/certExpirations/TableCertExpirationsController.js
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.
+ */
+
+var TableCertExpirationsController = function(tableName, certExpirations, dsXmlToIdMap, filter, $document, $scope, $state, $filter, locationUtils) {
+
+ /** All of the expiration fields converted to actual Dates */
+ $scope.certExpirations = certExpirations.map(
+ function(x) {
+ // need to convert this to a date object for ag-grid filter to work properly
+ x.expiration = new Date(x.expiration);
+ return x;
+ });
+
+ $scope.dsXmlToIdMap = dsXmlToIdMap;
+
+ $scope.editCertExpirations = function(dsId) {
+ locationUtils.navigateToPath('/delivery-services/' + dsId + '/ssl-keys');
+ }
+
+ /**
+ * Formats the contents of a 'federation' column cell as "True" or blank for visibility.
+ */
+ function federatedCellFormatter(params) {
+ if (!params.value) {
+ return '';
+ } else {
+ return params.value;
+ }
+ }
+
+ /** The columns of the ag-grid table */
+ $scope.columns = [
+ {
+ headerName: "Delivery Service",
+ field: "deliveryservice",
+ hide: false
+ },
+ {
+ headerName: "CDN",
+ field: "cdn",
+ hide: false
+ },
+ {
+ headerName: "Provider",
+ field: "provider",
+ hide: false
+ },
+ {
+ headerName: "Expiration",
+ field: "expiration",
+ hide: false,
+ filter: "agDateColumnFilter"
+ },
+ {
+ headerName: "Federated",
+ field: "federated",
+ hide: false,
+ valueFormatter: federatedCellFormatter
+ },
+ ];
+
+ /** Options, configuration, data and callbacks for the ag-grid table. */
+ $scope.gridOptions = {
+ onRowClick: function(params) {
+ const selection = window.getSelection().toString();
+ if(selection === "" || selection === $scope.mouseDownSelectionText) {
Review comment:
`$scope.mouseDownSelectionText` is either undefined or the empty string, and `window.getSelection.toString()` will always return a string. when `$scope.mouseDownSelectionText` is an empty string, the second part of this condition is redundant. `undefined` and `string` types have no overlap, so this condition will always evaluate to `false` in the case that `$scope.mouseDownSelectionText` is `undefined`. Furthermore, the empty string is the only false-y string value, so all in all this condition can be written as simply `if (!selection) {`.
--
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 change in pull request #6339: Cert expirations tp
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #6339:
URL: https://github.com/apache/trafficcontrol/pull/6339#discussion_r778348409
##########
File path: traffic_portal/app/src/common/modules/table/certExpirations/TableCertExpirationsController.js
##########
@@ -0,0 +1,318 @@
+/*
+ * 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.
+ */
+
+var TableCertExpirationsController = function(tableName, certExpirations, dsXmlToIdMap, filter, $document, $scope, $state, $filter, locationUtils, certExpirationsService) {
+
+ let table;
Review comment:
`table` is never referenced, and has no value
##########
File path: traffic_portal/app/src/common/modules/table/certExpirations/TableCertExpirationsController.js
##########
@@ -0,0 +1,318 @@
+/*
+ * 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.
+ */
+
+var TableCertExpirationsController = function(tableName, certExpirations, dsXmlToIdMap, filter, $document, $scope, $state, $filter, locationUtils, certExpirationsService) {
+
+ let table;
+
+ $scope.certExpirations = certExpirations;
+ $scope.dsXmlToIdMap = dsXmlToIdMap;
+ $scope.days;
Review comment:
This statement has no effect
##########
File path: traffic_portal/app/src/common/modules/table/certExpirations/table.certExpirations.tpl.html
##########
@@ -0,0 +1,70 @@
+<!--
+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.
+-->
+
+<div class="x_panel">
+ <div class="x_title">
+ <ol class="breadcrumb pull-left">
+ <li class="active">Certificate Expirations</li>
+ </ol>
+ <div class="pull-right">
+ <div class="form-inline" role="search">
+ <input id="quickSearch" name="quickSearch" type="search" class="form-control text-input" placeholder="Quick search..." ng-model="quickSearch" ng-change="onQuickSearchChanged()" aria-label="Search"/>
+ <div class="input-group text-input">
+ <span class="input-group-addon">
+ <label for="days">Days</label>
+ </span>
+ <input id="days" name="days" type="number" min="1" class="form-control" placeholder="" ng-model="days" ng-change="onDaysChanged(days)" aria-label="Days"/>
Review comment:
You don't need `aria-label` since the element is explicitly labeled. An empty `placeholder` is the default, so it need not be specified.
##########
File path: traffic_portal/app/src/common/modules/table/certExpirations/table.certExpirations.tpl.html
##########
@@ -0,0 +1,70 @@
+<!--
+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.
+-->
+
+<div class="x_panel">
+ <div class="x_title">
+ <ol class="breadcrumb pull-left">
+ <li class="active">Certificate Expirations</li>
+ </ol>
+ <div class="pull-right">
+ <div class="form-inline" role="search">
+ <input id="quickSearch" name="quickSearch" type="search" class="form-control text-input" placeholder="Quick search..." ng-model="quickSearch" ng-change="onQuickSearchChanged()" aria-label="Search"/>
+ <div class="input-group text-input">
+ <span class="input-group-addon">
+ <label for="days">Days</label>
+ </span>
+ <input id="days" name="days" type="number" min="1" class="form-control" placeholder="" ng-model="days" ng-change="onDaysChanged(days)" aria-label="Days"/>
Review comment:
Passing `days` to the `onDaysChanged` expands expressions of non-constant variables to scope properties; this is calling `$scope.onDaysChanged($scope.days)`. This is a bit of a nitpick, but since `$scope.days` is in-scope of the function `$scope.onDaysChanged`, you don't need to pass it an argument, you can just access the model value directly.
--
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] mattjackson220 commented on a change in pull request #6339: Cert expirations tp
Posted by GitBox <gi...@apache.org>.
mattjackson220 commented on a change in pull request #6339:
URL: https://github.com/apache/trafficcontrol/pull/6339#discussion_r771740984
##########
File path: traffic_portal/app/src/common/modules/table/certExpirations/TableCertExpirationsController.js
##########
@@ -0,0 +1,317 @@
+/*
+ * 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.
+ */
+
+var TableCertExpirationsController = function(tableName, certExpirations, filter, $document, $scope, $state, $filter, locationUtils, certExpirationsService) {
+
+ let table;
+
+ $scope.certExpirations = certExpirations;
+ $scope.days;
+
+ $scope.editCertExpirations = function(dsId) {
+ locationUtils.navigateToPath('/delivery-services/' + dsId + '/ssl-keys');
+ }
+
+ /**
+ * Formats the contents of a 'expiration' column cell as just the date.
+ */
+ function dateCellFormatter(params) {
+ return params.value ? $filter('date')(params.value, 'MM/dd/yyyy') : params.value;
+ }
+
+ /**
+ * Formats the contents of a 'expiration' column cell as just the date.
+ */
+ function federatedCellFormatter(params) {
+ if (!params.value) {
+ return '';
+ } else {
+ return params.value;
+ }
+ }
+
+ $scope.refresh = function() {
+ $state.reload(); // reloads all the resolves for the view
+ };
+
+ /** Toggles the visibility of a column that has the ID provided as 'col'. */
+ $scope.toggleVisibility = function(col) {
+ const visible = $scope.gridOptions.columnApi.getColumn(col).isVisible();
+ $scope.gridOptions.columnApi.setColumnVisible(col, !visible);
+ };
+
+ /** The columns of the ag-grid table */
+ const columns = [
+ {
+ headerName: "Delivery Service",
+ field: "deliveryservice",
+ hide: false
+ },
+ {
+ headerName: "CDN",
+ field: "cdn",
+ hide: false
+ },
+ {
+ headerName: "Provider",
+ field: "provider",
+ hide: false
+ },
+ {
+ headerName: "Expiration",
+ field: "expiration",
+ hide: false,
+ valueFormatter: dateCellFormatter
+ },
+ {
+ headerName: "Federated",
+ field: "federated",
+ hide: false,
+ valueFormatter: federatedCellFormatter
+ },
+ ];
+
+ $scope.pageSize = 100;
+
+ /** Options, configuration, data and callbacks for the ag-grid table. */
+ $scope.gridOptions = {
Review comment:
i tried that route originally but i couldnt get the `Days` input to be with the other inputs or to get it to update the table correctly with the new information. i can try again next week on that but i got the other updates in there
--
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 change in pull request #6339: Cert expirations tp
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #6339:
URL: https://github.com/apache/trafficcontrol/pull/6339#discussion_r781526922
##########
File path: traffic_portal/app/src/common/modules/table/certExpirations/TableCertExpirationsController.js
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.
+ */
+
+var TableCertExpirationsController = function(tableName, certExpirations, dsXmlToIdMap, filter, $document, $scope, $state, $filter, locationUtils) {
+
+ /** All of the expiration fields converted to actual Dates */
+ $scope.certExpirations = certExpirations.map(
+ function(x) {
+ // need to convert this to a date object for ag-grid filter to work properly
+ x.expiration = new Date(x.expiration);
+ return x;
+ });
+
+ $scope.dsXmlToIdMap = dsXmlToIdMap;
+
+ $scope.editCertExpirations = function(dsId) {
+ locationUtils.navigateToPath('/delivery-services/' + dsId + '/ssl-keys');
+ }
+
+ /**
+ * Formats the contents of a 'federation' column cell as "True" or blank for visibility.
+ */
+ function federatedCellFormatter(params) {
+ if (!params.value) {
+ return '';
+ } else {
+ return params.value;
+ }
+ }
+
+ /** The columns of the ag-grid table */
+ $scope.columns = [
+ {
+ headerName: "Delivery Service",
+ field: "deliveryservice",
+ hide: false
+ },
+ {
+ headerName: "CDN",
+ field: "cdn",
+ hide: false
+ },
+ {
+ headerName: "Provider",
+ field: "provider",
+ hide: false
+ },
+ {
+ headerName: "Expiration",
+ field: "expiration",
+ hide: false,
+ filter: "agDateColumnFilter"
+ },
+ {
+ headerName: "Federated",
+ field: "federated",
+ hide: false,
+ valueFormatter: federatedCellFormatter
+ },
+ ];
+
+ /** Options, configuration, data and callbacks for the ag-grid table. */
+ $scope.gridOptions = {
+ onRowClick: function(params) {
+ const selection = window.getSelection().toString();
+ if(selection === "" || selection === $scope.mouseDownSelectionText) {
+ locationUtils.navigateToPath('/delivery-services/' + $scope.dsXmlToIdMap[params.data.deliveryservice] + '/ssl-keys');
+ // Event is outside the digest cycle, so we need to trigger one.
+ $scope.$apply();
+ }
+ $scope.mouseDownSelectionText = "";
Review comment:
Since this value is never (usefully) read, this line can be removed.
##########
File path: traffic_portal/app/src/common/modules/table/certExpirations/TableCertExpirationsController.js
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.
+ */
+
+var TableCertExpirationsController = function(tableName, certExpirations, dsXmlToIdMap, filter, $document, $scope, $state, $filter, locationUtils) {
+
+ /** All of the expiration fields converted to actual Dates */
+ $scope.certExpirations = certExpirations.map(
+ function(x) {
+ // need to convert this to a date object for ag-grid filter to work properly
+ x.expiration = new Date(x.expiration);
+ return x;
+ });
+
+ $scope.dsXmlToIdMap = dsXmlToIdMap;
+
+ $scope.editCertExpirations = function(dsId) {
+ locationUtils.navigateToPath('/delivery-services/' + dsId + '/ssl-keys');
+ }
+
+ /**
+ * Formats the contents of a 'federation' column cell as "True" or blank for visibility.
+ */
+ function federatedCellFormatter(params) {
+ if (!params.value) {
+ return '';
+ } else {
+ return params.value;
+ }
Review comment:
This will make it always 'true' or blank - shouldn't it be 'true' or 'false'?
##########
File path: traffic_portal/app/src/common/modules/table/certExpirations/TableCertExpirationsController.js
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.
+ */
+
+var TableCertExpirationsController = function(tableName, certExpirations, dsXmlToIdMap, filter, $document, $scope, $state, $filter, locationUtils) {
Review comment:
`filter`, `$document`, `$state`, and `$filter` are all unused
##########
File path: traffic_portal/app/src/common/modules/table/certExpirations/TableCertExpirationsController.js
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.
+ */
+
+var TableCertExpirationsController = function(tableName, certExpirations, dsXmlToIdMap, filter, $document, $scope, $state, $filter, locationUtils) {
+
+ /** All of the expiration fields converted to actual Dates */
+ $scope.certExpirations = certExpirations.map(
+ function(x) {
+ // need to convert this to a date object for ag-grid filter to work properly
+ x.expiration = new Date(x.expiration);
+ return x;
+ });
+
+ $scope.dsXmlToIdMap = dsXmlToIdMap;
+
+ $scope.editCertExpirations = function(dsId) {
+ locationUtils.navigateToPath('/delivery-services/' + dsId + '/ssl-keys');
+ }
+
+ /**
+ * Formats the contents of a 'federation' column cell as "True" or blank for visibility.
+ */
+ function federatedCellFormatter(params) {
+ if (!params.value) {
+ return '';
+ } else {
+ return params.value;
+ }
+ }
+
+ /** The columns of the ag-grid table */
+ $scope.columns = [
+ {
+ headerName: "Delivery Service",
+ field: "deliveryservice",
+ hide: false
+ },
+ {
+ headerName: "CDN",
+ field: "cdn",
+ hide: false
+ },
+ {
+ headerName: "Provider",
+ field: "provider",
+ hide: false
+ },
+ {
+ headerName: "Expiration",
+ field: "expiration",
+ hide: false,
+ filter: "agDateColumnFilter"
+ },
+ {
+ headerName: "Federated",
+ field: "federated",
+ hide: false,
+ valueFormatter: federatedCellFormatter
+ },
+ ];
+
+ /** Options, configuration, data and callbacks for the ag-grid table. */
+ $scope.gridOptions = {
+ onRowClick: function(params) {
+ const selection = window.getSelection().toString();
+ if(selection === "" || selection === $scope.mouseDownSelectionText) {
Review comment:
`$scope.mouseDownSelectionText` is either undefined or the empty string, and `window.getSelection.toString()` will always return a string. when `$scope.mouseDownSelectionText` is an empty string, the second part of this condition is redundant. `undefined` and `string` types have no overlap, so this condition will always evaluate to `false` in the case that `$scope.mouseDownSelectionText` is `undefined`.. Furthermore, the empty string is the only false-y string value, so all in all this condition can be written as simply `if (!selection) {`.
##########
File path: traffic_portal/test/integration/PageObjects/SideNavigationPage.po.ts
##########
@@ -157,6 +158,11 @@ export class SideNavigationPage extends BasePage{
await browser.actions().mouseMove(this.lnkServiceCategories).perform();
await browser.actions().click(this.lnkServiceCategories).perform();
}
+ async NavigateToCertExpirationsPage(){
+ await browser.wait(ExpectedConditions.visibilityOf(this.lnkCertExpirations), 2000);
+ await browser.actions().mouseMove(this.lnkCertExpirations).perform();
+ await browser.actions().click(this.lnkCertExpirations).perform();
Review comment:
I don't think the part about moving the mouse is under test here, so we can just do this on one line with
```
return this.lnkCertExpirations.click();
```
This method is also (like most others) missing a return type (should be `Promise<void>`), and IMO ought to have a visibility modifier (`public`) and jsDoc comment (but neither of those are enforced, so don't feel like you have to do them - return type though is something I've been requesting on all callables, if not actually enforced).
##########
File path: traffic_portal/app/src/modules/private/certExpirations/list/index.js
##########
@@ -0,0 +1,55 @@
+/*
+ * 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.
+ */
+
+module.exports = angular.module('trafficPortal.private.certExpirations.list', [])
+ .config(function($stateProvider, $urlRouterProvider) {
+ $stateProvider
+ .state('trafficPortal.private.certExpirations.list', {
+ url: '',
+ views: {
+ certExpirationsContent: {
+ templateUrl: 'common/modules/table/certExpirations/table.certExpirations.tpl.html',
+ controller: 'TableCertExpirationsController',
+ resolve: {
+ certExpirations: function(certExpirationsService) {
+ return certExpirationsService.getCertExpirations();
+ },
+ tableName: function() {
+ return 'cert-expirations';
+ },
+ filter: function() {
+ return null;
+ },
Review comment:
I'm not sure what this is for, but it's never used in the controller for this state, so it should just be removed.
##########
File path: traffic_portal/app/src/modules/private/certExpirations/list/index.js
##########
@@ -0,0 +1,55 @@
+/*
+ * 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.
+ */
+
+module.exports = angular.module('trafficPortal.private.certExpirations.list', [])
+ .config(function($stateProvider, $urlRouterProvider) {
+ $stateProvider
+ .state('trafficPortal.private.certExpirations.list', {
+ url: '',
+ views: {
+ certExpirationsContent: {
+ templateUrl: 'common/modules/table/certExpirations/table.certExpirations.tpl.html',
+ controller: 'TableCertExpirationsController',
+ resolve: {
+ certExpirations: function(certExpirationsService) {
+ return certExpirationsService.getCertExpirations();
+ },
+ tableName: function() {
+ return 'cert-expirations';
+ },
+ filter: function() {
+ return null;
+ },
+ deliveryservices: function(deliveryServiceService) {
+ return deliveryServiceService.getDeliveryServices();
+ },
+ dsXmlToIdMap: function (deliveryservices) {
+ var result = {};
+ for (i in deliveryservices) {
+ result[deliveryservices[i].xmlId] = deliveryservices[i].id;
+ }
+ return result;
+ }
Review comment:
This doesn't represent a state; it's pure functional logic. This should probably be moved to the controller instead of injected as a dependency thereof.
Also, `result` is never reassigned, so it could be `const`, and can in fact be constructed as a mapping on the `deliveryservices` collection like so:
```javascript
$scope.dsXMLIDToIDMap = Object.fromEntries(deliveryservices.map(ds=>[ds.xmlId, ds.id]));
```
It can also be made into an ES6 Map in much the same way, if that interests you at all:
```javascript
$scope.dsXMLIDToIDMap = new Map(deliveryservices.map(ds=>[ds.xmlId, ds.id]));
```
##########
File path: traffic_portal/app/src/common/modules/table/certExpirations/TableCertExpirationsController.js
##########
@@ -0,0 +1,94 @@
+/*
+ * 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.
+ */
+
+var TableCertExpirationsController = function(tableName, certExpirations, dsXmlToIdMap, filter, $document, $scope, $state, $filter, locationUtils) {
+
+ /** All of the expiration fields converted to actual Dates */
+ $scope.certExpirations = certExpirations.map(
+ function(x) {
+ // need to convert this to a date object for ag-grid filter to work properly
+ x.expiration = new Date(x.expiration);
+ return x;
+ });
+
+ $scope.dsXmlToIdMap = dsXmlToIdMap;
+
+ $scope.editCertExpirations = function(dsId) {
+ locationUtils.navigateToPath('/delivery-services/' + dsId + '/ssl-keys');
+ }
+
+ /**
+ * Formats the contents of a 'federation' column cell as "True" or blank for visibility.
+ */
+ function federatedCellFormatter(params) {
+ if (!params.value) {
+ return '';
+ } else {
+ return params.value;
+ }
+ }
+
+ /** The columns of the ag-grid table */
+ $scope.columns = [
+ {
+ headerName: "Delivery Service",
+ field: "deliveryservice",
+ hide: false
+ },
+ {
+ headerName: "CDN",
+ field: "cdn",
+ hide: false
+ },
+ {
+ headerName: "Provider",
+ field: "provider",
+ hide: false
+ },
+ {
+ headerName: "Expiration",
+ field: "expiration",
+ hide: false,
+ filter: "agDateColumnFilter"
+ },
+ {
+ headerName: "Federated",
+ field: "federated",
+ hide: false,
+ valueFormatter: federatedCellFormatter
+ },
+ ];
+
+ /** Options, configuration, data and callbacks for the ag-grid table. */
+ $scope.gridOptions = {
+ onRowClick: function(params) {
+ const selection = window.getSelection().toString();
+ if(selection === "" || selection === $scope.mouseDownSelectionText) {
+ locationUtils.navigateToPath('/delivery-services/' + $scope.dsXmlToIdMap[params.data.deliveryservice] + '/ssl-keys');
+ // Event is outside the digest cycle, so we need to trigger one.
+ $scope.$apply();
+ }
+ $scope.mouseDownSelectionText = "";
+ }
+ };
+
+};
+
+TableCertExpirationsController.$inject = ['tableName', 'certExpirations', 'dsXmlToIdMap', 'filter', '$document', '$scope', '$state', '$filter', 'locationUtils'];
Review comment:
same as above RE unused dependency injections.
--
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 change in pull request #6339: Cert expirations tp
Posted by GitBox <gi...@apache.org>.
ocket8888 commented on a change in pull request #6339:
URL: https://github.com/apache/trafficcontrol/pull/6339#discussion_r744000153
##########
File path: traffic_portal/app/src/common/modules/table/certExpirations/TableCertExpirationsController.js
##########
@@ -0,0 +1,317 @@
+/*
+ * 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.
+ */
+
+var TableCertExpirationsController = function(tableName, certExpirations, filter, $document, $scope, $state, $filter, locationUtils, certExpirationsService) {
+
+ let table;
Review comment:
indentation is inconsistent in this file, some lines uses spaces and some lines use the One True indentation. There's no mandate on which to use throughout TP, but you should pick one to stick with for the file to prevent unintentionally massive diffs if the file is ever changed again by someone who's editor consolidates the indentation to a single style (fwiw tabs are mandated in TPv2, so it's a solid choice IMO).
##########
File path: traffic_portal/app/src/common/api/index.js
##########
@@ -24,7 +24,8 @@ module.exports = angular.module('trafficPortal.api', [])
.service('cacheStatsService', require('./CacheStatsService'))
.service('capabilityService', require('./CapabilityService'))
.service('cdnService', require('./CDNService'))
- .service('changeLogService', require('./ChangeLogService'))
+ .service('certExpirationsService', require('./CertExpirationsService'))
+ .service('changeLogService', require('./ChangeLogService'))
Review comment:
same as above RE: indentation
##########
File path: traffic_portal/app/src/app.js
##########
@@ -372,6 +374,7 @@ var trafficPortal = angular.module('trafficPortal', [
require('./common/modules/table/cdnNotifications').name,
require('./common/modules/table/cdnProfiles').name,
require('./common/modules/table/cdnServers').name,
+ require('./common/modules/table/certExpirations').name,
Review comment:
the rest of the file (unfortunately) uses spaces for indentation, as do your other two added lines. If you wanna change the indentation of the file to use actual indentation, I'd support that fully. As-is, though, this should conform.
##########
File path: traffic_portal/app/src/common/modules/table/certExpirations/TableCertExpirationsController.js
##########
@@ -0,0 +1,317 @@
+/*
+ * 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.
+ */
+
+var TableCertExpirationsController = function(tableName, certExpirations, filter, $document, $scope, $state, $filter, locationUtils, certExpirationsService) {
+
+ let table;
+
+ $scope.certExpirations = certExpirations;
+ $scope.days;
+
+ $scope.editCertExpirations = function(dsId) {
+ locationUtils.navigateToPath('/delivery-services/' + dsId + '/ssl-keys');
+ }
+
+ /**
+ * Formats the contents of a 'expiration' column cell as just the date.
+ */
+ function dateCellFormatter(params) {
+ return params.value ? $filter('date')(params.value, 'MM/dd/yyyy') : params.value;
+ }
+
+ /**
+ * Formats the contents of a 'expiration' column cell as just the date.
+ */
+ function federatedCellFormatter(params) {
+ if (!params.value) {
+ return '';
+ } else {
+ return params.value;
+ }
+ }
+
+ $scope.refresh = function() {
+ $state.reload(); // reloads all the resolves for the view
+ };
+
+ /** Toggles the visibility of a column that has the ID provided as 'col'. */
+ $scope.toggleVisibility = function(col) {
+ const visible = $scope.gridOptions.columnApi.getColumn(col).isVisible();
+ $scope.gridOptions.columnApi.setColumnVisible(col, !visible);
+ };
+
+ /** The columns of the ag-grid table */
+ const columns = [
+ {
+ headerName: "Delivery Service",
+ field: "deliveryservice",
+ hide: false
+ },
+ {
+ headerName: "CDN",
+ field: "cdn",
+ hide: false
+ },
+ {
+ headerName: "Provider",
+ field: "provider",
+ hide: false
+ },
+ {
+ headerName: "Expiration",
+ field: "expiration",
+ hide: false,
+ valueFormatter: dateCellFormatter
+ },
+ {
+ headerName: "Federated",
+ field: "federated",
+ hide: false,
+ valueFormatter: federatedCellFormatter
+ },
+ ];
+
+ $scope.pageSize = 100;
+
+ /** Options, configuration, data and callbacks for the ag-grid table. */
+ $scope.gridOptions = {
Review comment:
It looks like you might be doing more work here than you need to. Can you make use of the existing AG-Grid table component?
##########
File path: traffic_portal/test/integration/PageObjects/SideNavigationPage.po.ts
##########
@@ -36,6 +36,7 @@ export class SideNavigationPage extends BasePage{
private lnkDeliveryServices = element(by.xpath("//a[@href='/#!/delivery-services']"));
private lnkDeliveryServiceRequest = element(by.xpath("//a[@href='/#!/delivery-service-requests']"));
private lnkServiceCategories = element(by.xpath("//a[@href='/#!/service-categories']"));
+ private lnkCertExpirations = element(by.xpath("//a[@href='/#!/cert-expirations']"));
Review comment:
CSS selectors are much more familiar and easy to read than XPath, should use those instead
--
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