Skip to content

Commit

Permalink
Merge branch 'main' into task/WP-273-CategoryIcon
Browse files Browse the repository at this point in the history
  • Loading branch information
tjgrafft authored Oct 10, 2023
2 parents d924916 + 4cacf32 commit b47aab0
Show file tree
Hide file tree
Showing 16 changed files with 105 additions and 47 deletions.
9 changes: 8 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

## [3.2.1] - 2023-10-05: Search and Target Path fixes

### Fixed
WP-297: Fix site search for public and community data (#870)
WP-306: Fix target path regression (#871)

## [3.2.0] - 2023-10-02: V3 integration improvements; bug fixes

### Added
Expand Down Expand Up @@ -953,7 +959,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [1.0.0] - 2020-02-28
v1.0.0 Production release as of Feb 28, 2020.

[unreleased]: https://github.com/TACC/Core-Portal/compare/v3.1.2...HEAD
[unreleased]: https://github.com/TACC/Core-Portal/compare/v3.2.1...HEAD
[3.2.1]: https://github.com/TACC/Core-Portal/releases/tag/v3.2.1
[3.2.0]: https://github.com/TACC/Core-Portal/releases/tag/v3.2.0
[3.1.2]: https://github.com/TACC/Core-Portal/releases/tag/v3.1.2
[3.1.1]: https://github.com/TACC/Core-Portal/releases/tag/v3.1.1
Expand Down
11 changes: 7 additions & 4 deletions client/src/components/Applications/AppForm/AppForm.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import { Link } from 'react-router-dom';
import { getSystemName } from 'utils/systems';
import FormSchema from './AppFormSchema';
import {
checkAndSetDefaultTargetPath,
isTargetPathEmpty,
isTargetPathField,
getInputFieldFromTargetPathField,
getQueueMaxMinutes,
Expand Down Expand Up @@ -514,9 +514,12 @@ export const AppSchemaForm = ({ app }) => {
.flat()
.filter((fileInput) => fileInput.sourceUrl) // filter out any empty values
.map((fileInput) => {
fileInput.targetPath = checkAndSetDefaultTargetPath(
fileInput.targetPath
);
if (isTargetPathEmpty(fileInput.targetPath)) {
return {
name: fileInput.name,
sourceUrl: fileInput.sourceUrl,
};
}
return fileInput;
});

Expand Down
23 changes: 19 additions & 4 deletions client/src/components/Applications/AppForm/AppFormUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,20 +202,35 @@ export const getInputFieldFromTargetPathField = (targetPathFieldName) => {
};

/**
* Sets the default value if target path is not set.
* Check if targetPath is empty on input field
*
* @function
* @param {String} targetPathFieldValue
* @returns {String} target path value
* @returns {boolean} if target path is empty
*/
export const checkAndSetDefaultTargetPath = (targetPathFieldValue) => {
export const isTargetPathEmpty = (targetPathFieldValue) => {
if (targetPathFieldValue === null || targetPathFieldValue === undefined) {
return '*';
return true;
}

targetPathFieldValue = targetPathFieldValue.trim();

if (targetPathFieldValue.trim() === '') {
return true;
}

return false;
};

/**
* Sets the default value if target path is not set.
*
* @function
* @param {String} targetPathFieldValue
* @returns {String} target path value
*/
export const checkAndSetDefaultTargetPath = (targetPathFieldValue) => {
if (isTargetPathEmpty(targetPathFieldValue)) {
return '*';
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,7 @@ const DataFilesProjectMembers = ({
};

DataFilesProjectMembers.propTypes = {
projectId: PropTypes.string,
members: PropTypes.arrayOf(
PropTypes.shape({
username: PropTypes.string,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const DataFilesAddButton = ({ readOnly }) => {
payload: { operation: 'upload', props: {} },
});
};
const err = useSelector((state) => state.files.error.FilesListing);
const hasError = useSelector((state) => state.files.error.FilesListing);
const { api, scheme } = useSelector(
(state) => state.files.params.FilesListing
);
Expand Down Expand Up @@ -60,7 +60,7 @@ const DataFilesAddButton = ({ readOnly }) => {

const disabled =
readOnly ||
err !== false ||
hasError !== false ||
api !== 'tapis' ||
(scheme !== 'private' && scheme !== 'projects');

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,14 @@
.dropdown-menu {
border-color: var(--global-color-accent--normal);
border-radius: 0;
margin-top: 11px;
padding: 0;
margin: 11px 3px 0;
width: 200px;
vertical-align: top;
}
.dropdown-menu::before {
position: absolute;
top: -10px;
left: 65px;
left: 67px;
border-right: 10px solid transparent;
border-bottom: 10px solid var(--global-color-accent--normal);
border-left: 10px solid transparent;
Expand All @@ -42,7 +41,7 @@
.dropdown-menu::after {
position: absolute;
top: -9px;
left: 66px;
left: 68px;
border-right: 9px solid transparent;
border-bottom: 9px solid #ffffff;
border-left: 9px solid transparent;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,18 @@ const OPERATION_MAP = {
case 'upload':
case 'move':
case 'copy': {
const destPath = response.path.split('/').slice(0, -1).join('/');
const projectName = findProjectTitle(projectList, response.systemId);
const { path, systemId, source } = response;
const destPath = path.split('/').slice(0, -1).join('/');
const projectName = findProjectTitle(projectList, systemId);

let op = mappedOp;

if (mappedOp === 'copied') {
const srcSystem =
response.source.split('/')[0] === 'https:'
? response.source.split('/')[7]
: response.source.split('/')[2];
const destSystem = response.systemId;
source.split('/')[0] === 'https:'
? source.split('/')[7]
: source.split('/')[2];
const destSystem = systemId;
if (srcSystem !== destSystem) {
op = 'started copying';
}
Expand All @@ -62,7 +63,7 @@ const OPERATION_MAP = {
} else {
dest =
destPath === '/' || destPath === ''
? `${findSystemDisplayName(systemList, response.systemId)}/`
? `${findSystemDisplayName(systemList, systemId)}/`
: destPath;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { DropdownSelector } from '_common';
import styles from './DataFilesSystemSelector.module.scss';

const DataFilesSystemSelector = ({
systemId,
systemAndHomeDirId,
section,
disabled,
operation,
Expand All @@ -17,11 +17,12 @@ const DataFilesSystemSelector = ({
(state) => state.systems.storage.configuration
);
const modalProps = useSelector((state) => state.files.modalProps[operation]);
const findSystem = (id) =>
const findSystem = (systemAndHomeDirPath) =>
systemList.find(
(system) => `${system.system}${system.homeDir || ''}` === id
(system) =>
`${system.system}${system.homeDir || ''}` === systemAndHomeDirPath
);
const [selectedSystem, setSelectedSystem] = useState(systemId);
const [selectedSystem, setSelectedSystem] = useState(systemAndHomeDirId);

const openSystem = useCallback(
(event) => {
Expand Down Expand Up @@ -69,7 +70,7 @@ const DataFilesSystemSelector = ({
};

useEffect(() => {
setSelectedSystem(systemId);
setSelectedSystem(systemAndHomeDirId);
}, []);

const dropdownSystems = systemList.filter(
Expand Down Expand Up @@ -111,7 +112,7 @@ const DataFilesSystemSelector = ({
};

DataFilesSystemSelector.propTypes = {
systemId: PropTypes.string,
systemAndHomeDirId: PropTypes.string,
section: PropTypes.string.isRequired,
disabled: PropTypes.bool,
operation: PropTypes.string.isRequired,
Expand All @@ -120,7 +121,7 @@ DataFilesSystemSelector.propTypes = {
};

DataFilesSystemSelector.defaultProps = {
systemId: '',
systemAndHomeDirId: '',
disabled: false,
showProjects: false,
excludedSystems: [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@ import { bool, func, string } from 'prop-types';
import './JobsSessionModal.global.scss';
import styles from './JobsSessionModal.module.scss';

const JobsSessionModal = ({ isOpen, toggle, interactiveSessionLink }) => {
const JobsSessionModal = ({
isOpen,
toggle,
interactiveSessionLink,
message,
}) => {
return (
<Modal isOpen={isOpen} toggle={toggle} contentClassName="session-modal">
<ModalHeader
Expand All @@ -18,6 +23,7 @@ const JobsSessionModal = ({ isOpen, toggle, interactiveSessionLink }) => {
<span>
Click the button below to connect to the interactive session.
</span>
{message && <b>{message}</b>}
<span>To end the job, quit the application within the session.</span>
<span>
Files may take some time to appear in the output location after the
Expand Down
3 changes: 3 additions & 0 deletions client/src/components/Jobs/JobsStatus/JobsStatus.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ function JobsStatus({ status, fancy, jobUuid }) {

const notifs = useSelector((state) => state.notifications.list.notifs);
let interactiveSessionLink;
let message;

const jobConcluded = isTerminalState(status) || status === 'ARCHIVING';

Expand All @@ -85,6 +86,7 @@ function JobsStatus({ status, fancy, jobUuid }) {
);
const notif = interactiveNotifs.find((n) => n.extra.uuid === jobUuid);
interactiveSessionLink = notif ? notif.action_link : null;
message = notif ? notif.message : null;
}

return (
Expand All @@ -109,6 +111,7 @@ function JobsStatus({ status, fancy, jobUuid }) {
toggle={toggleModal}
isOpen={modal}
interactiveSessionLink={interactiveSessionLink}
message={message}
/>
</>
)}
Expand Down
8 changes: 4 additions & 4 deletions server/poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,11 @@ def migrate_project(project_id):

for co_pi in v2_project.co_pis.all():
v2_role = get_role(project_id, co_pi.username)
v3_role = ROLE_MAP[v2_role]
try:
v3_role = ROLE_MAP[v2_role]
except KeyError:
print(f'ERROR: No role found for: {v2_role}')
v3_role = "reader"
try:
add_user_to_workspace(client, project_id, co_pi.username, v3_role)
except NotFoundError:
Expand All @@ -71,7 +75,11 @@ def migrate_project(project_id):

for team_member in v2_project.team_members.all():
v2_role = get_role(project_id, team_member.username)
v3_role = ROLE_MAP[v2_role]
try:
v3_role = ROLE_MAP[v2_role]
except KeyError:
print(f'ERROR: No role found for: {v2_role}')
v3_role = "reader"
try:
add_user_to_workspace(client, project_id, team_member.username, v3_role)
except NotFoundError:
Expand Down
16 changes: 11 additions & 5 deletions server/portal/apps/site_search/api/unit_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ def mock_cms_search(mocker):
yield mocked_fn


@pytest.fixture
def mock_service_account(mocker):
yield mocker.patch('portal.apps.site_search.api.views.service_account', autospec=True)


@pytest.fixture
def mock_files_search(mocker):
mocked_fn = mocker.patch('portal.apps.site_search.api.views.files_search')
Expand Down Expand Up @@ -76,7 +81,7 @@ def test_search_with_auth(regular_user, client, mock_cms_search,
'include': True}}


def test_search_no_auth(client, mock_cms_search, mock_files_search):
def test_search_no_auth(client, mock_cms_search, mock_files_search, mock_service_account):
response = client.get('/api/site-search/?page=0&query_string=test')

assert response.json() == {
Expand All @@ -93,7 +98,7 @@ def test_search_no_auth(client, mock_cms_search, mock_files_search):


def test_search_public(client, configure_public, mock_cms_search,
mock_files_search):
mock_files_search, mock_service_account):
response = client.get('/api/site-search/?page=0&query_string=test')

assert response.json() == {
Expand Down Expand Up @@ -132,15 +137,16 @@ def test_cms_search_util(mock_dsl_search):
'highlight': {'body': ['highlight 1']}}])


def test_file_search_util(mock_file_search):
def test_file_search_util(mock_file_search, regular_user):
from portal.apps.site_search.api.views import files_search
mock_file_search.return_value = {'count': 1,
'listing':
[{'name': 'testfile',
'path': '/path/to/testfile'}]}
res = files_search('test_query', 'test_system')
client = regular_user.tapis_oauth.client
res = files_search(client, 'test_query', 'test_system')

mock_file_search.assert_called_with(None, 'test_system', '/',
mock_file_search.assert_called_with(client, 'test_system', '/',
query_string='test_query',
filter=None,
offset=0,
Expand Down
Loading

0 comments on commit b47aab0

Please sign in to comment.