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.

Sign up for GitHub

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

WIP - Add ability to generate report without uploading #849

Closed
wants to merge 2 commits into from
Closed

WIP - Add ability to generate report without uploading #849

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f542634
Add ability to generate report without uploading
chris1984 Oct 17, 2023
7c1d0d6
Get JS tests to run
chris1984 Oct 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/controllers/concerns/inventory_upload/report_actions.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ def initialize(**params)
end
end

def start_report_generation(organization_id)
def start_report_generation(organization_id, disconnected)
ForemanTasks.async_task(ForemanInventoryUpload::Async::GenerateReportJob, ForemanInventoryUpload.generated_reports_folder, organization_id)
end

Expand Down
3 changes: 2 additions & 1 deletion app/controllers/foreman_inventory_upload/reports_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ def last

def generate
organization_id = params[:organization_id]
disconnected = params[:disconnected]

start_report_generation(organization_id)
start_report_generation(organization_id, disconnected)

render json: {
action_status: 'success',
Expand Down
4 changes: 2 additions & 2 deletions lib/foreman_inventory_upload/async/generate_report_job.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ def self.output_label(label)
"report_for_#{label}"
end

def plan(base_folder, organization_id)
def plan(base_folder, organization_id, disconnected)
sequence do
super(
GenerateReportJob.output_label(organization_id),
Expand All @@ -18,7 +18,7 @@ def plan(base_folder, organization_id)
base_folder,
ForemanInventoryUpload.facts_archive_name(organization_id),
organization_id
)
) unless disconnected
end
end

Expand Down
34 changes: 34 additions & 0 deletions webpack/ForemanInventoryUpload/Components/AccountList/AccountListActions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,40 @@ export const stopAccountStatusPolling = pollingProcessID => dispatch => {
});
};

export const restartDisconnected = (accountID, activeTab) => async dispatch => {
let processController = null;
let processStatusName = null;
let disconnected = true

if (activeTab === 'uploading') {
processController = 'uploads';
processStatusName = 'upload_report_status';
} else {
processController = 'reports';
processStatusName = 'generate_report_status';
}

try {
await API.post(inventoryUrl(`${accountID}/${processController}`));
dispatch({
type: INVENTORY_PROCESS_RESTART,
payload: {
accountID,
disconnected,
processStatusName,
},
});
} catch (error) {
dispatch(
addToast({
sticky: true,
type: 'error',
message: error.message,
})
);
}
};

export const restartProcess = (accountID, activeTab) => async dispatch => {
let processController = null;
let processStatusName = null;
Expand Down
4 changes: 2 additions & 2 deletions webpack/ForemanInventoryUpload/Components/Dashboard/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { bindActionCreators } from 'redux';
import { connect } from 'react-redux';

import * as actions from './DashboardActions';
import { restartProcess } from '../AccountList/AccountListActions';
import { restartProcess, restartDisconnected } from '../AccountList/AccountListActions';
import reducer from './DashboardReducer';

import Dashboard from './Dashboard';
Expand All @@ -25,7 +25,7 @@ const mapStateToProps = (state, { accountID }) => ({

// map action dispatchers to props
const mapDispatchToProps = dispatch =>
bindActionCreators({ ...actions, restartProcess }, dispatch);
bindActionCreators({ ...actions, restartProcess, restartDisconnected }, dispatch);

// export reducers
export const reducers = { dashboard: reducer };
Expand Down
2 changes: 2 additions & 0 deletions webpack/ForemanInventoryUpload/Components/ReportGenerate/ReportGenerate.fixtures.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ export const exitCode = 'exit 0';
export const logs = ['No running process'];
export const completed = 0;
export const restartProcess = noop;
export const restartDisconnected = noop;
export const error = null;
export const scheduled = '2019-08-21T16:14:16.520+03:00';
export const props = {
exitCode,
logs,
completed,
restartProcess,
restartDisconnected,
error,
scheduled,
};
4 changes: 4 additions & 0 deletions webpack/ForemanInventoryUpload/Components/ReportGenerate/ReportGenerate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@ const ReportGenerate = ({
completed,
error,
restartProcess,
restartDisconnected,
toggleFullScreen,
scheduled,
}) => (
<TabContainer className="report-generate">
<TabHeader
exitCode={exitCode}
onRestart={restartProcess}
restartDisconnected={restartDisconnected}
toggleFullScreen={toggleFullScreen}
/>
<TabBody
Expand All @@ -39,6 +41,7 @@ ReportGenerate.propTypes = {
]),
completed: PropTypes.number,
error: PropTypes.string,
restartDisconnected: PropTypes.func,
restartProcess: PropTypes.func,
toggleFullScreen: PropTypes.func,
scheduled: PropTypes.string,
Expand All @@ -50,6 +53,7 @@ ReportGenerate.defaultProps = {
completed: 0,
error: null,
restartProcess: noop,
restartDisconnected: noop,
toggleFullScreen: noop,
scheduled: null,
};
Expand Down
2 changes: 2 additions & 0 deletions webpack/ForemanInventoryUpload/Components/ReportUpload/ReportUpload.fixtures.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ export const exitCode = 'exit 0';
export const logs = ['No running process'];
export const completed = 0;
export const restartProcess = noop;
export const restartDisconnected = noop;
export const downloadReports = noop;
export const error = null;
export const props = {
Expand All @@ -13,6 +14,7 @@ export const props = {
logs,
completed,
restartProcess,
restartDisconnected,
downloadReports,
error,
};
49 changes: 40 additions & 9 deletions webpack/ForemanInventoryUpload/Components/TabHeader/TabHeader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,26 +1,57 @@
import React from 'react';
import PropTypes from 'prop-types';
import { Grid, Button, Icon } from 'patternfly-react';
import { Grid, Button, Icon, DropdownItem, Dropdown, DropdownToggle, DropdownToggleAction } from 'patternfly-react';
import { noop } from 'foremanReact/common/helpers';
import { sprintf, translate as __ } from 'foremanReact/common/I18n';
import { isExitCodeLoading } from '../../ForemanInventoryHelpers';
import './tabHeader.scss';

const TabHeader = ({ exitCode, onRestart, onDownload, toggleFullScreen }) => (
const onActionToggle = () => {
setIsActionOpen(prev => !prev);
Copy link
Member

Choose a reason for hiding this comment

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

Where is this setter coming from?

I think it would be better to use React's useState inside the TabHeader component to trigger a component re-render

Copy link
Member Author

@chris1984 chris1984 Oct 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I copied and pasted that in from the Katello Errata tab, since we needed an onActionToggle That was probably a bad idea, but I am guessing we are missing something else.

};

const dropdownItems = [
<DropdownItem
aria-label="restart_disconnected"
ouiaId="restart_disconnected"
key="restart_disconnected"
component="button"
onClick={restartDisconnected}
Copy link
Member

Choose a reason for hiding this comment

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

in case you would still like to use Redux in this PR,
the prop restartDisconnected would be passed to the TabHeader component, maybe you want to pass the dropdownItems into that component, e.g:

const TabHeader = ({ exitCode, onRestart, onDownload, restartDisconnected, toggleFullScreen }) => {
const dropdownItems = [....]
return <Grid.Row className="tab-header">.......</Grid.Row>
}

Copy link
Member Author

Choose a reason for hiding this comment

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

When I put in const dropdownItems = [....] everything lights up red like a stop light 🤣 the error from vscode says

Identifier expected. 'const' is a reserved word that cannot be used here.ts(1359)

isDisabled={isExitCodeLoading(exitCode)}
>
{__('Restart without uploading')}
</DropdownItem>,
];

const TabHeader = ({ exitCode, onRestart, onDownload, restartDisconnected, toggleFullScreen }) => (
<Grid.Row className="tab-header">
<Grid.Col sm={6}>
<p>{sprintf(__('Exit Code: %s'), exitCode)}</p>
</Grid.Col>
<Grid.Col sm={6}>
<div className="tab-action-buttons">
{onRestart ? (
<Button
bsStyle="primary"
onClick={onRestart}
disabled={isExitCodeLoading(exitCode)}
>
{__('Restart')}
</Button>
<Dropdown
aria-label="restart_dropdown"
ouiaId="restart_dropdown"
toggle={
<DropdownToggle
aria-label="restart_report_toggle"
ouiaId="restart_report_toggle"
splitButtonItems={[
<DropdownToggleAction key="action" aria-label="bulk_actions" onClick={onRestart}>
{__('Restart')}
</DropdownToggleAction>,
]}
splitButtonVariant="action"
toggleVariant="primary"
onToggle={onActionToggle}
isDisabled={isExitCodeLoading(exitCode)}
/>
}
isOpen={isActionOpen}
dropdownItems={dropdownItems}
/>
Comment on lines +34 to +54
Copy link
Member

Choose a reason for hiding this comment

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

You have already done a lot of work to fit this in, but if interested this could have been implemented in a separate component with "newer" technologies even without using redux actions reducers and all that stuff.

for the API call you could use useAPI

import { useAPI } from 'foremanReact/common/hooks/API/APIHooks';

) : null}
{onDownload ? (
<Button onClick={onDownload}>
Expand Down
Loading