-
Notifications
You must be signed in to change notification settings - Fork 64
Delete Server in Tornjak Manager #676
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
base: main
Are you sure you want to change the base?
Changes from all commits
1f0bac4
149ce1a
aaa4f8f
d7f62ba
dc6000f
6a61957
c072515
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,15 +1,17 @@ | ||||||||||||||||||||||||||||||||
| import React from "react"; | ||||||||||||||||||||||||||||||||
| import { connect } from 'react-redux'; | ||||||||||||||||||||||||||||||||
| // import IsManager from 'components/is_manager'; | ||||||||||||||||||||||||||||||||
| import IsManager from 'components/is_manager'; | ||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||
| serversListUpdateFunc | ||||||||||||||||||||||||||||||||
| } from 'redux/actions'; | ||||||||||||||||||||||||||||||||
| import Table from './list-table'; | ||||||||||||||||||||||||||||||||
| import { ServersList } from "components/types"; | ||||||||||||||||||||||||||||||||
| // import { DenormalizedRow } from "carbon-components-react"; | ||||||||||||||||||||||||||||||||
| import { DenormalizedRow } from "carbon-components-react"; | ||||||||||||||||||||||||||||||||
| import { RootState } from "redux/reducers"; | ||||||||||||||||||||||||||||||||
| import TornjakApi from 'components/tornjak-api-helpers'; | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| // ServersListTable takes in | ||||||||||||||||||||||||||||||||
| // listTableData: servers data to be rendered on table | ||||||||||||||||||||||||||||||||
| // returns servers data inside a carbon component table with specified functions | ||||||||||||||||||||||||||||||||
|
|
@@ -50,7 +52,7 @@ class ServersListTable extends React.Component<ServersListTableProp, ServersList | |||||||||||||||||||||||||||||||
| listTableData: [], | ||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||
| this.prepareTableData = this.prepareTableData.bind(this); | ||||||||||||||||||||||||||||||||
| // this.deleteServer = this.deleteServer.bind(this); | ||||||||||||||||||||||||||||||||
| this.deleteServer = this.deleteServer.bind(this); | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| componentDidMount() { | ||||||||||||||||||||||||||||||||
|
|
@@ -85,30 +87,25 @@ class ServersListTable extends React.Component<ServersListTableProp, ServersList | |||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| //Note: future implementation - server delete function | ||||||||||||||||||||||||||||||||
| // keep code | ||||||||||||||||||||||||||||||||
| // deleteServer(selectedRows: readonly DenormalizedRow[]) { | ||||||||||||||||||||||||||||||||
| // if (!selectedRows || selectedRows.length === 0) return ""; | ||||||||||||||||||||||||||||||||
| // let server: { name: string }[] = [], successMessage | ||||||||||||||||||||||||||||||||
| deleteServer(selectedRows: readonly DenormalizedRow[]) { | ||||||||||||||||||||||||||||||||
| if (!selectedRows || selectedRows.length === 0) return ""; | ||||||||||||||||||||||||||||||||
| if (!IsManager) return ""; | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
| if (!IsManager) return ""; | |
| if (!IsManager()) return ""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Incorrect. agent-list.tsx also uses IsManager as a value, not as a function. The type of IsManager is a boolean, its not a function.
Copilot
AI
Nov 4, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable server[i] may have incorrect value in async callback. Since this is inside an async callback and i is captured by reference, by the time the callback executes, i will likely equal selectedRows.length. Consider storing server[i].name in a local constant before the async call or use forEach to properly capture the value.
| successMessage = this.TornjakApi.serverDelete({ server: server[i] }, this.props.serversListUpdateFunc, this.props.globalServersList); | |
| successMessage.then(function (result) { | |
| if (result === "SUCCESS") { | |
| window.alert(`SERVER "${server[i].name}" DELETED SUCCESSFULLY!`); | |
| window.location.reload(); | |
| } else { | |
| window.alert(`Error deleting server "${server[i].name}": ` + result); | |
| const serverName = server[i].name; | |
| successMessage = this.TornjakApi.serverDelete({ server: server[i] }, this.props.serversListUpdateFunc, this.props.globalServersList); | |
| successMessage.then(function (result) { | |
| if (result === "SUCCESS") { | |
| window.alert(`SERVER "${serverName}" DELETED SUCCESSFULLY!`); | |
| window.location.reload(); | |
| } else { | |
| window.alert(`Error deleting server "${serverName}": ` + result); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The filter logic is incorrect. It compares
el.name(a string) withinputData(an object containing{ server: { name: string } }). This should beel.name !== inputData.server.nameto correctly filter out the deleted server.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works either/both ways. Should we stick to the current codebase pattern of checking
el.name !== inputData?