Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change rendering/behavior of ILO/Mgmt IPs in Traffic Portal Servers list #5332

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

MylesBock
Copy link
Contributor

@MylesBock MylesBock commented Nov 24, 2020

What does this PR (Pull Request) do?

Updates TableServersController to change the behavior for the ag-grid.
A new renderer is introduced and the previous SSHCellRenderer has been updated to use a div instead of an a tag
The corresponding onRowClick behavior was also updated to ignore navigation when these columns are clicked

Which Traffic Control components are affected by this PR?

  • Traffic Portal

What is the best way to verify this PR?

Fire up TP with this branch, ensure ILO columns open to the iLO interface via HTTPS and the 'normal' mgmt IPs open a tab with an SSH target. Also ensure that when either of these are clicked the page remains the same.

If this is a bug fix, what versions of Traffic Control are affected?

The following criteria are ALL met by this PR

  • This PR includes tests OR I have explained why tests are unnecessary
  • This PR includes documentation OR I have explained why documentation is unnecessary
  • This PR includes an update to CHANGELOG.md OR such an update is not necessary
  • This PR includes any and all required license headers
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY (see the Apache Software Foundation's security guidelines for details)

- added new cell renderer (ILOCellRenderer)
- added new click handler for SSHCellRenderers and ILOCellRenderers
- both ssh/ilo cell renderers are now divs instead of a elements
- a value associated with the cell has been added to both indicating what href to open
- openCellInNewWindow() updated to use this value
- onRowClick updated to not navigate if column's cells have either of these renderers
@mitchell852
Copy link
Member

thanks @MylesBock - maybe @shamrickus can review?

@mitchell852 mitchell852 added Traffic Portal v1 related to Traffic Portal version 1 regression bug a bug in existing functionality introduced by a new version labels Nov 24, 2020
@MylesBock
Copy link
Contributor Author

no problem, I would say so ag-grid is not my forte for sure

Copy link
Member

@shamrickus shamrickus left a comment

Choose a reason for hiding this comment

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

This PR should have an entry in CHANGELOG.md

@@ -365,7 +379,16 @@ var TableServersController = function(tableName, servers, filter, $scope, $state
},
onRowClicked: function(params) {
const selection = window.getSelection().toString();
if(selection === "" || selection === $scope.mouseDownSelectionText) {
const overrideCells = [
Copy link
Member

Choose a reason for hiding this comment

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

I'd move this to a field on the controller since it never changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

10-4, this was added mostly last minute to ensure that the default behavior for the row wasn't applied

this.eGui.textContent = params.value;
this.data.cellHrefValue = "ssh://" + userModel.user.username + "@" + params.value;
Copy link
Member

Choose a reason for hiding this comment

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

Getting this.data is undefined on page load

Uncaught TypeError: this.data is undefined
    init https://localhost/resources/assets/js/app.js?built=1606856003402:24254
    initComponent https://localhost/resources/assets/js/ag-grid-community/dist/ag-grid-community.min.js:8
    createAndInitUserComponent https://localhost/resources/assets/js/ag-grid-community/dist/ag-grid-community.min.js:8
    newCellRenderer https://localhost/resources/assets/js/ag-grid-community/dist/ag-grid-community.min.js:8
    createCellRendererFunc https://localhost/resources/assets/js/ag-grid-community/dist/ag-grid-community.min.js:8
    executeFrame https://localhost/resources/assets/js/ag-grid-community/dist/ag-grid-community.min.js:8
    requestFrame https://localhost/resources/assets/js/ag-grid-community/dist/ag-grid-community.min.js:8
app.js:24254:3```

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This literally crossed my mind Saturday afternoon 🙃

I need to figure out where it's approps to stash this, the context presently isn't correct of course

"ipGateway",
"ipAddress"
];
let columnId = params.column.getColId();
Copy link
Member

Choose a reason for hiding this comment

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

Getting params.column is undefined

};
SSHCellRenderer.prototype.getGui = function() {return this.eGui;};

function ILOCellRenderer() {}
ILOCellRenderer.prototype.init = function(params) {
this.eGui = document.createElement("div");
Copy link
Member

Choose a reason for hiding this comment

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

ILOCellRenderer never gets initialized. I believe it's because all of the columns it's on are hidden by default (the SSHCellRenderer has one that is not, so it does get initialized).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤔 I'll dig into this one tomorrow

@MylesBock MylesBock marked this pull request as draft December 20, 2020 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression bug a bug in existing functionality introduced by a new version Traffic Portal v1 related to Traffic Portal version 1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TO pre 5.0 server list, network ip and ilo ip were clickable without going into server details.
3 participants