WP-66: refactor data files components 2

[asimregmi]

Overview

Updated systemId prop to systemAndHomeDirId in all of the the DataFiles components where it is used to make the prop more descriptive.

Related

• WP-66

Changes

• Updated prop names in DataFilesSelectModal.jsx and DataFilesCopyModal.jsx

Testing

1. Go to DataFiles and test all DataFiles functionalities Copy
2. Go to Applications and submit a job using OpenSeesSP or similar where you can select inputs (SelectModal is used here).

UI

No visible UI change

Notes

Previous PR was #876. In this PR, updated prop names to two more files.

[codecov]

Codecov Report

Merging #885 (1c41821) into main (d7a003e) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #885   +/-   ##
=======================================
  Coverage   63.44%   63.44%           
=======================================
  Files         427      427           
  Lines       12215    12215           
  Branches     2510     2510           
=======================================
  Hits         7750     7750           
  Misses       4259     4259           
  Partials      206      206           

Flag	Coverage Δ
javascript	69.67% <ø> (ø)
unittests	57.20% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...s/DataFiles/DataFilesModals/DataFilesCopyModal.jsx	76.27% <ø> (ø)
...DataFiles/DataFilesModals/DataFilesSelectModal.jsx	71.87% <ø> (ø)
...ataFilesProjectMembers/DataFilesProjectMembers.jsx	56.00% <ø> (ø)

[nathanfranklin]

LGTM 👍

I've spent a little time to see if we can catch this in linting or unit testing. I looked over https://github.com/jsx-eslint/eslint-plugin-react but can't find any rule that matches this scenario (i.e. using a prop that doesn't exist). sounds like there is no rule due to the use case where a React component might want to forward extra props to child components.

My only other comment to consider, before mering, is marking the property projectId from https://github.com/TACC/Core-Portal/pull/876/files#r1350565668 isRequired (and thanks for catching that the property was missing 💯 ).

[nathanfranklin]

Out of scope for here (and it wouldn't have caught systemId/systemAndHomeDirId), but it feels like we should configure eslintrc to be more strict on props, like:

    "react/prop-types": 2,
    "react/no-unused-prop-types": 2,

[asimregmi]

Thank you!! Added the PropType isRequired for ProjectId.

I went into the rabbit hole to see if there was a library we could use with Jest. It might be possible with Enzyme and a complex test to catch if a component is using a prop that is undefined (doesn't exist, like systemId in this case, after being renamed to systemAndHomeDirId). Not sure if this will be efficient, however.

[edmondsgarrett]

Looks good, toolbar functions and input selector working!