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

[Prod] Bug fixes; communication log query; increase the size of the sequelize connection pool #2246

Merged
merged 21 commits into from
Jul 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
1498bd8
Create communication-logs.sql
GarrettEHill Jun 27, 2024
4ca9ce9
remove space
GarrettEHill Jun 27, 2024
804b212
ensure its an array
nvms Jun 28, 2024
b95d236
update test
nvms Jun 28, 2024
637517b
Fix unassociated label issue
thewatermethod Jun 28, 2024
d8d661d
Update communication-logs.sql
GarrettEHill Jun 28, 2024
6704b6f
Merge pull request #2236 from HHS/TTAHUB-3141/ssdi-CommunicationLog-e…
GarrettEHill Jun 28, 2024
fddc379
Update config.js
GarrettEHill Jun 29, 2024
f314c59
Merge pull request #2237 from HHS/jp/3114/bugfix
nvms Jul 1, 2024
1dc9d8e
Merge pull request #2240 from HHS/mb/TTAHUB/fix-form-label-in-rtr
thewatermethod Jul 1, 2024
4d17654
handle exceptions in goal policy
thewatermethod Jul 1, 2024
3f3eeb2
Update goal permissions to allow admins to delete goals
thewatermethod Jul 1, 2024
e17794a
Add test
thewatermethod Jul 1, 2024
a3999aa
Adjust cron run time and add logging
kryswisnaskas Jul 1, 2024
08090a7
Remove extra space
kryswisnaskas Jul 1, 2024
1584fea
Merge pull request #2241 from HHS/TTAHUB-3143/increase-pool-size
GarrettEHill Jul 1, 2024
a15d93b
Add a mock
kryswisnaskas Jul 1, 2024
2bc4294
Merge pull request #2245 from HHS/kw-adjust-cron-start
kryswisnaskas Jul 1, 2024
ff92640
Merge pull request #2244 from HHS/mb/TTAHUB-3117/better-exception-han…
thewatermethod Jul 2, 2024
6b3bc92
Merge remote-tracking branch 'origin/main' into mb/TTAHUB-3134/adjust…
thewatermethod Jul 2, 2024
d1a6027
Merge pull request #2243 from HHS/mb/TTAHUB-3134/adjust-goal-permissions
thewatermethod Jul 2, 2024
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 config/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ module.exports = {
ssl: true,
},
pool: {
max: 10,
max: 30,
validate: connectionValidation,
},
},
Expand Down
30 changes: 19 additions & 11 deletions frontend/src/components/GoalForm/Form.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,17 +136,25 @@ export default function Form({
collaborators={collaborators}
/>

<GrantSelect
selectedGrants={selectedGrants}
isOnReport={isOnReport}
setSelectedGrants={setSelectedGrants}
possibleGrants={possibleGrants}
validateGrantNumbers={validateGrantNumbers}
error={errors[FORM_FIELD_INDEXES.GRANTS]}
isLoading={isAppLoading}
goalStatus={status}
userCanEdit={userCanEdit}
/>
<FormFieldThatIsSometimesReadOnly
permissions={[
status !== 'Closed',
userCanEdit,
possibleGrants.length > 1,
!isOnReport,
]}
label="Recipient grant numbers"
value={selectedGrants.map((grant) => grant.numberWithProgramTypes).join(', ')}
>
<GrantSelect
selectedGrants={selectedGrants}
setSelectedGrants={setSelectedGrants}
possibleGrants={possibleGrants}
validateGrantNumbers={validateGrantNumbers}
error={errors[FORM_FIELD_INDEXES.GRANTS]}
isLoading={isAppLoading}
/>
</FormFieldThatIsSometimesReadOnly>

<GoalName
goalName={goalName}
Expand Down
58 changes: 22 additions & 36 deletions frontend/src/components/GoalForm/GrantSelect.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useMemo } from 'react';
import React from 'react';
import PropTypes from 'prop-types';
import Select from 'react-select';
import {
Expand All @@ -13,50 +13,39 @@ import Req from '../Req';
export default function GrantSelect({
error,
selectedGrants,
isOnReport,
setSelectedGrants,
possibleGrants,
validateGrantNumbers,
inputName,
label,
isLoading,
goalStatus,
userCanEdit,
}) {
const cannotEdit = useMemo(() => isOnReport || goalStatus === 'Closed' || possibleGrants.length === 1 || !userCanEdit, [goalStatus, isOnReport, possibleGrants.length, userCanEdit]);

return (
<FormGroup error={error.props.children}>
<Label htmlFor={inputName} className={cannotEdit ? 'text-bold' : ''}>
<Label htmlFor={inputName}>
{label}
{' '}
{!isOnReport ? <Req /> : null }
<Req />
</Label>
{cannotEdit ? (
<p className="margin-top-0 usa-prose">{selectedGrants.map((grant) => grant.numberWithProgramTypes).join(', ')}</p>
) : (
<>
{error}
<Select
placeholder=""
inputId={inputName}
onChange={setSelectedGrants}
options={possibleGrants}
styles={selectOptionsReset}
components={{
DropdownIndicator: null,
}}
className="usa-select"
closeMenuOnSelect={false}
value={selectedGrants}
isMulti
onBlur={() => validateGrantNumbers(SELECT_GRANTS_ERROR)}
isDisabled={isLoading}
getOptionLabel={(option) => option.numberWithProgramTypes}
getOptionValue={(option) => option.id}
/>
</>
)}
{error}
<Select
placeholder=""
inputId={inputName}
onChange={setSelectedGrants}
options={possibleGrants}
styles={selectOptionsReset}
components={{
DropdownIndicator: null,
}}
className="usa-select"
closeMenuOnSelect={false}
value={selectedGrants}
isMulti
onBlur={() => validateGrantNumbers(SELECT_GRANTS_ERROR)}
isDisabled={isLoading}
getOptionLabel={(option) => option.numberWithProgramTypes}
getOptionValue={(option) => option.id}
/>
</FormGroup>
);
}
Expand All @@ -67,7 +56,6 @@ GrantSelect.propTypes = {
label: PropTypes.string,
value: PropTypes.number,
})).isRequired,
isOnReport: PropTypes.bool.isRequired,
setSelectedGrants: PropTypes.func.isRequired,
possibleGrants: PropTypes.arrayOf(PropTypes.shape({
label: PropTypes.string,
Expand All @@ -77,8 +65,6 @@ GrantSelect.propTypes = {
inputName: PropTypes.string,
label: PropTypes.string,
isLoading: PropTypes.bool,
goalStatus: PropTypes.string.isRequired,
userCanEdit: PropTypes.bool.isRequired,
};

GrantSelect.defaultProps = {
Expand Down
7 changes: 2 additions & 5 deletions frontend/src/components/GoalForm/__tests__/GrantSelect.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,18 @@ import GrantSelect from '../GrantSelect';

describe('GrantSelect', () => {
const renderGrantSelect = (
validateGrantNumbers = jest.fn(), userCanEdit = true, selectedGrants = [],
validateGrantNumbers = jest.fn(),
) => {
render((
<div>
<GrantSelect
error={<></>}
setSelectedGrants={jest.fn()}
selectedGrants={selectedGrants}
selectedGrants={[]}
validateGrantNumbers={validateGrantNumbers}
label="Select grants"
inputName="grantSelect"
isLoading={false}
isOnReport={false}
goalStatus="Not Started"
userCanEdit={userCanEdit}
possibleGrants={[
{
value: 1,
Expand Down
6 changes: 3 additions & 3 deletions src/lib/cron.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ import { logger, auditLogger } from '../logger';
// Run at 4 am ET
const schedule = '0 4 * * *';
// Run daily at 4 pm
const dailySched = '0 16 * * 1-5';
const dailySched = '1 16 * * 1-5';
// Run at 4 pm every Friday
const weeklySched = '0 16 * * 5';
const weeklySched = '5 16 * * 5';
// Run at 4 pm on the last of the month
const monthlySched = '0 16 28-31 * *';
const monthlySched = '10 16 28-31 * *';
const timezone = 'America/New_York';

const runJob = () => {
Expand Down
6 changes: 6 additions & 0 deletions src/models/activityReportCollaborator.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const { Model } = require('sequelize');
const { auditLogger } = require('../logger');
const generateFullName = require('./helpers/generateFullName');

export default (sequelize, DataTypes) => {
Expand Down Expand Up @@ -27,6 +28,11 @@ export default (sequelize, DataTypes) => {
fullName: {
type: DataTypes.VIRTUAL,
get() {
if (!this.user) {
const { stack } = new Error();
auditLogger.error(`Access attempt to undefined user for userID: ${this.userId}, stack: ${stack}`);
return '';
}
const roles = this.roles && this.roles.length
? this.roles : this.user.roles;
return generateFullName(this.user.name, roles);
Expand Down
4 changes: 2 additions & 2 deletions src/models/hooks/sessionReportPilot.js
Original file line number Diff line number Diff line change
Expand Up @@ -308,15 +308,15 @@ export const createGoalsForSessionRecipientsIfNecessary = async (sequelize, sess
export const removeGoalsForSessionRecipientsIfNecessary = async (sequelize, sessionReportOrInstance, options) => {
const processSessionReport = async (sessionReport) => {
let event;
let nextSessionRecipients;
let nextSessionRecipients = [];

if (sessionReport && sessionReport.data) {
const data = (typeof sessionReport.data.val === 'string')
? JSON.parse(sessionReport.data.val)
: sessionReport.data;

event = data.event;
nextSessionRecipients = data.recipients;
nextSessionRecipients = data.recipients || [];
}

if (!event || !event.id || !sessionReport || !sessionReport.id) return;
Expand Down
4 changes: 3 additions & 1 deletion src/models/hooks/sessionReportPilot.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,9 @@ describe('removeGoalsForSessionRecipientsIfNecessary hook', () => {
id: '1',
data: {
event: { id: '2' },
recipients: [],
// Leave this commented verifies the bugfix in TTAHUB-3114, where
// nextSessionRecipients was never assigned a good value.
// recipients: [],
},
})),
},
Expand Down
24 changes: 24 additions & 0 deletions src/models/tests/activityReportCollaborator.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { ActivityReportCollaborator } from '..';
import { auditLogger } from '../../logger';

jest.mock('../../logger');

describe('ActivityReportCollaborator Model', () => {
it('should handle undefined user gracefully', () => {
const instance = new ActivityReportCollaborator();
instance.user = undefined;
instance.userId = 1;

expect(() => instance.fullName).not.toThrow();
expect(auditLogger.error).toHaveBeenCalled();
expect(instance.fullName).toBe('');
});

it('should return correct fullName when user and roles are defined', () => {
const instance = new ActivityReportCollaborator();
instance.user = { name: 'Joe Brown', roles: [{ name: 'Admin' }] };
instance.roles = [{ name: 'GS' }];

expect(instance.fullName).toEqual('Joe Brown, GS');
});
});
10 changes: 9 additions & 1 deletion src/policies/goals.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,11 @@ export default class Goal {
)
&& permission.regionId === region),
);
return !isUndefined(permissions);

// eslint-disable-next-line max-len
const isAdmin = find(this.user.permissions, (permission) => permission.scopeId === SCOPES.ADMIN);

return !isUndefined(isAdmin) || !isUndefined(permissions);
}

// refactored to take a region id rather than directly check
Expand Down Expand Up @@ -103,7 +107,11 @@ export default class Goal {
}

canView() {
if (!this.goal || !this.goal.grant) {
return false;
}
const region = this.goal.grant.regionId;

return this.canReadInRegion(region);
}
}
67 changes: 66 additions & 1 deletion src/policies/goals.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ describe('Goals policies', () => {
const goal = {
objectives: [],
grant: { regionId: 2 },

};
const user = {
permissions: [
Expand All @@ -90,6 +89,24 @@ describe('Goals policies', () => {
const policy = new Goal(user, goal);
expect(policy.canDelete()).toBe(true);
});

it('returns true if user is admin', async () => {
const goal = {
objectives: [],
grant: { regionId: 2 },
};
const user = {
permissions: [
{
regionId: 14,
scopeId: SCOPES.ADMIN,
},
],
};

const policy = new Goal(user, goal);
expect(policy.canDelete()).toBe(true);
});
});

describe('canCreate', () => {
Expand Down Expand Up @@ -216,4 +233,52 @@ describe('Goals policies', () => {
expect(policy.isOnApprovedActivityReports()).toBe(true);
});
});

describe('canView', () => {
it('returns false if no goal', () => {
const user = {
permissions: [
{
regionId: 2,
scopeId: SCOPES.APPROVE_REPORTS,
},
],
};

const policy = new Goal(user);
expect(policy.canView()).toBe(false);
});

it('returns false if goal has no grant', () => {
const user = {
permissions: [
{
regionId: 2,
scopeId: SCOPES.APPROVE_REPORTS,
},
],
};

const policy = new Goal(user, {});
expect(policy.canView()).toBe(false);
});

it('returns true if user has permissions in that region', () => {
const user = {
permissions: [
{
regionId: 2,
scopeId: SCOPES.APPROVE_REPORTS,
},
],
};

const goal = {
grant: { regionId: 2 },
};

const policy = new Goal(user, goal);
expect(policy.canView()).toBe(true);
});
});
});
Loading