Skip to content

Commit

Permalink
Caceves/appeals 53755 code climate fixes (#22427)
Browse files Browse the repository at this point in the history
* Code climate fixes initial commit

* Added proptypes to tooltiphelper file

* Jest fixes

* Updated jest snap shots

* Fix for dentical blocks of code being found in multiple locations for radio input field

* Lint fixes

* Jest fixes

* Jest fixes updating the snapshots

* Codeclimate fixes for const props

* testing the duplication error fix for jest

* User tests 'FeatureToggle.enabled?(:correspondence_queue)' at least 3 times

* Add ignore for pdf offense

* Complexity fixes

---------

Co-authored-by: divyadasari-va <divya.dasari@va.gov>
Co-authored-by: divyadasari-va <135847343+divyadasari-va@users.noreply.github.com>
  • Loading branch information
3 people authored Aug 8, 2024
1 parent 63165da commit eca8845
Show file tree
Hide file tree
Showing 16 changed files with 3,771 additions and 1,598 deletions.
4 changes: 2 additions & 2 deletions app/controllers/concerns/correspondence_controller_concern.rb
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ def build_multi_error_message(errors, action_prefix)
end

def error_reason(error)
return "" unless error
return "" unless error.is_a?(String)

case error
when Constants.CORRESPONDENCE_AUTO_ASSIGN_ERROR.NOD_ERROR then "of NOD permissions settings"
Expand All @@ -148,7 +148,7 @@ def build_single_error_message(action_prefix, reason)
def build_error_message(*args)
# Build error message for multiple correspondence based on error types
message = "#{args[0]} cases were not #{args[1]}assigned to user"
message = "• #{message}" if use_bullet
message = "• #{message}" if args[3].present?
message += " because #{args[2]}." unless args[0].zero?
message
end
Expand Down
10 changes: 7 additions & 3 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def hearings_user?
end

def inbound_ops_team_superuser?
return false unless FeatureToggle.enabled?(:correspondence_queue)
return false unless correspondence_queue_enabled?

member_of_organization?(InboundOpsTeam.singleton) &&
OrganizationUserPermissionChecker.new.can?(
Expand All @@ -110,14 +110,14 @@ def inbound_ops_team_superuser?

# check for user that is not an admin of the inbound ops team
def inbound_ops_team_user?
return false unless FeatureToggle.enabled?(:correspondence_queue)
return false unless correspondence_queue_enabled?

organizations.include?(InboundOpsTeam.singleton) &&
!inbound_ops_team_supervisor?
end

def inbound_ops_team_supervisor?
return false unless FeatureToggle.enabled?(:correspondence_queue)
return false unless correspondence_queue_enabled?

administered_teams.include?(InboundOpsTeam.singleton)
end
Expand Down Expand Up @@ -591,6 +591,10 @@ def system_user?

private

def correspondence_queue_enabled?
FeatureToggle.enabled?(:correspondence_queue)
end

def inactive_judge_team
JudgeTeam.unscoped.inactive.find_by(id: organizations_users.admin.pluck(:organization_id))
end
Expand Down
54 changes: 18 additions & 36 deletions client/app/components/RadioField.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ import classNames from 'classnames';

import RequiredIndicator from './RequiredIndicator';
import StringUtil from '../util/StringUtil';
import Tooltip from './Tooltip';
import MaybeAddTooltip from './TooltipHelper';

import ACD_LEVERS from '../../constants/ACD_LEVERS';
import RadioInput from './RadioInput';
import { extractFieldProps } from './fieldUtils';

import { helpText } from './RadioField.module.scss';

Expand All @@ -29,11 +30,9 @@ RadioFieldHelpText.propTypes = {
*/

export const RadioField = (props) => {

const { id, className, label, inputRef } = extractFieldProps(props);
const {
id,
className,
label,
inputRef,
inputProps,
name,
options,
Expand Down Expand Up @@ -68,25 +67,6 @@ export const RadioField = (props) => {
</span>
);

const maybeAddTooltip = (option, radioField) => {
if (option.tooltipText) {
const keyId = `tooltip-${option.value}`;

return <Tooltip
key={keyId}
id={keyId}
text={option.tooltipText}
position="right"
className="cf-radio-option-tooltip"
offset={{ right: 15 }}
>
{radioField}
</Tooltip>;
}

return radioField;
};

const isDisabled = (option) => Boolean(option.disabled);

const handleChange = (event) => onChange?.(event.target.value);
Expand All @@ -109,17 +89,15 @@ export const RadioField = (props) => {
className="cf-form-radio-option"
key={`${idPart}-${option.value}-${i}`}
>
<input
onChange={handleChange}
<RadioInput
handleChange={handleChange}
name={name}
type={ACD_LEVERS.data_types.radio}
id={`${idPart}_${option.value}`}
value={option.value}
// eslint-disable-next-line no-undefined
checked={controlled ? value === option.value : undefined}
disabled={optionDisabled}
ref={inputRef}
{...inputProps}
idPart={idPart}
option={option}
controlled={controlled}
value={value}
inputRef={inputRef}
inputProps={inputProps}
/>
<label
className={optionDisabled ? 'disabled' : ''}
Expand All @@ -131,7 +109,11 @@ export const RadioField = (props) => {
</div>
);

return maybeAddTooltip(option, radioField);
return (
<MaybeAddTooltip key={option.value} option={option}>
{radioField}
</MaybeAddTooltip>
);
})}
</div>
</fieldset>
Expand Down
54 changes: 18 additions & 36 deletions client/app/components/RadioFieldWithChildren.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,10 @@ import classNames from 'classnames';

import RequiredIndicator from './RequiredIndicator';
import StringUtil from '../util/StringUtil';
import Tooltip from './Tooltip';
import MaybeAddTooltip from './TooltipHelper';

import ACD_LEVERS from '../../constants/ACD_LEVERS';
import RadioInput from './RadioInput';
import { extractFieldProps } from './fieldUtils';

import { helpText } from './RadioField.module.scss';
const RadioFieldHelpText = ({ help, className }) => {
Expand All @@ -31,11 +32,9 @@ RadioFieldHelpText.propTypes = {

export const RadioFieldWithChildren = (props) => {

const { id, className, label, inputRef } = extractFieldProps(props);

const {
id,
className,
label,
inputRef,
inputProps,
name,
options,
Expand Down Expand Up @@ -70,25 +69,6 @@ export const RadioFieldWithChildren = (props) => {
</span>
);

const maybeAddTooltip = (option, radioField) => {
if (option.tooltipText) {
const keyId = `tooltip-${option.value}`;

return <Tooltip
key={keyId}
id={keyId}
text={option.tooltipText}
position="right"
className="cf-radio-option-tooltip"
offset={{ right: 15 }}
>
{radioField}
</Tooltip>;
}

return radioField;
};

const isDisabled = (option) => Boolean(option.disabled);

const handleChange = (event) => onChange?.(event.target.value);
Expand All @@ -111,17 +91,15 @@ export const RadioFieldWithChildren = (props) => {
className="cf-form-radio-option"
key={`${idPart}-${option.value}-${i}`}
>
<input
onChange={handleChange}
<RadioInput
handleChange={handleChange}
name={name}
type={ACD_LEVERS.data_types.radio}
id={`${idPart}_${option.value}`}
value={option.value}
// eslint-disable-next-line no-undefined
checked={controlled ? value === option.value : undefined}
disabled={optionDisabled}
ref={inputRef}
{...inputProps}
idPart={idPart}
option={option}
controlled={controlled}
value={value}
inputRef={inputRef}
inputProps={inputProps}
/>
<label
className={optionDisabled ? 'disabled' : ''}
Expand All @@ -134,7 +112,11 @@ export const RadioFieldWithChildren = (props) => {
</div>
);

return maybeAddTooltip(option, radioField);
return (
<MaybeAddTooltip key={option.value} option={option}>
{radioField}
</MaybeAddTooltip>
);
})}
</div>
</fieldset>
Expand Down
37 changes: 37 additions & 0 deletions client/app/components/RadioInput.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import React from 'react';
import PropTypes from 'prop-types';
import ACD_LEVERS from '../../constants/ACD_LEVERS';

const RadioInput = ({ handleChange, name, idPart, option, controlled, value, inputRef, inputProps }) => {
const isChecked = controlled ? value === option.value : option.checked;

return (
<input
onChange={handleChange}
name={name}
type={ACD_LEVERS.data_types.radio}
id={`${idPart}_${option.value}`}
value={option.value}
checked={isChecked}
disabled={Boolean(option.disabled)}
ref={inputRef}
{...inputProps}
/>
);
};

RadioInput.propTypes = {
handleChange: PropTypes.func.isRequired,
name: PropTypes.string.isRequired,
idPart: PropTypes.string.isRequired,
option: PropTypes.object.isRequired,
controlled: PropTypes.bool.isRequired,
value: PropTypes.oneOfType([PropTypes.string, PropTypes.bool]).isRequired,
inputRef: PropTypes.oneOfType([
PropTypes.func,
PropTypes.shape({ current: PropTypes.instanceOf(Element) }),
]),
inputProps: PropTypes.object,
};

export default RadioInput;
34 changes: 34 additions & 0 deletions client/app/components/TooltipHelper.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import React from 'react';
import PropTypes from 'prop-types';
import Tooltip from './Tooltip';

const MaybeAddTooltip = ({ option, children }) => {
if (!option.tooltipText) {
return children;
}

const keyId = `tooltip-${option.value}`;

return (
<Tooltip
key={keyId}
id={keyId}
text={option.tooltipText}
position="right"
className="cf-radio-option-tooltip"
offset={{ right: 15 }}
>
{children}
</Tooltip>
);
};

MaybeAddTooltip.propTypes = {
option: PropTypes.shape({
tooltipText: PropTypes.string,
value: PropTypes.string.isRequired,
}).isRequired,
children: PropTypes.node.isRequired,
};

export default MaybeAddTooltip;
4 changes: 4 additions & 0 deletions client/app/components/fieldUtils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export const extractFieldProps = (props) => {
const { id, className, label, inputRef } = props;
return { id, className, label, inputRef };
};
8 changes: 4 additions & 4 deletions client/app/queue/correspondence/CorrespondenceTaskRows.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,11 @@ const isCancelled = (task) => {
return task.status === TASK_STATUSES.cancelled;
};

const establishmentTask = (task) => {
const establishmentTaskCorrespondence = (task) => {
return task.type === 'EstablishmentTask';
};

const tdClassNames = (timeline, task) => {
const tdClassNamesforCorrespondence = (timeline, task) => {
const containerClass = timeline ? taskInfoWithIconTimelineContainer : '';
const closedAtClass = task.closedAt ? null : <span className="greyDotTimelineStyling"></span>;

Expand Down Expand Up @@ -174,7 +174,7 @@ class CorrespondenceTaskRows extends React.PureComponent {
<div className="cf-row-wrapper">
{taskInstructionsVisible && (
<React.Fragment key={`${task.assignedOn}${task.label}`}>
{!establishmentTask(task) &&
{!establishmentTaskCorrespondence(task) &&
<dt style={{ width: '100%' }}>
{COPY.TASK_SNAPSHOT_TASK_INSTRUCTIONS_LABEL}
</dt>
Expand Down Expand Up @@ -251,7 +251,7 @@ class CorrespondenceTaskRows extends React.PureComponent {
</td>
<td
{...taskInfoWithIconContainer}
className={tdClassNames(timeline, task)}
className={tdClassNamesforCorrespondence(timeline, task)}
>
{isCancelled(task) ? <CancelIcon /> : closedAtIcon(task, timeline)}

Expand Down
Loading

0 comments on commit eca8845

Please sign in to comment.