Skip to content

Commit

Permalink
GDB-10471 multi file upload fix (#1462)
Browse files Browse the repository at this point in the history
* ## What
Fix for the selection of files for import.
This also fixes the regress caused by the previous fix for the https://ontotext.atlassian.net/browse/GDB-10397 issue.

## Why
It appears that there are cases where files list maintained in the import view controller is not properly updated when a selection is made through the checkboxes in the resource tree which leads to wrong files sent for import.

## How
Added subscription for the selectedFileNamesUpdated event and updated the selectedForImportFiles list in the import view controller.
The subscription is implemented in the server import tab controller because it affects the import files selection and should not affect the user data tab.

* Removed the redundant selectedForImportFiles variable and replaced with direct references to selections from the context or the uploaded files depending on the currently rendered tab.
  • Loading branch information
svilenvelikov authored Jul 9, 2024
1 parent 5350149 commit 827462b
Showing 1 changed file with 34 additions and 44 deletions.
78 changes: 34 additions & 44 deletions src/js/angular/import/controllers/import-view.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,6 @@ importViewModule.controller('ImportViewCtrl', ['$scope', 'toastr', '$interval',
$scope.files = []; // should be private
$scope.fileChecked = {};
$scope.activeTabId = ImportContextService.getActiveTabId();
/**
* Contains a mapping of file names to flags indicating whether the file is selected or not.
* The selected files from this mapping are synchronized in the resources model each time it is refreshed from
* the server.
* @type {{[string]:boolean}}
*/
$scope.selectedForImportFiles = {};
$scope.popoverTemplateUrl = 'settingsPopoverTemplate.html';
$scope.fileFormatsExtended = FileFormats.getFileFormatsExtended();
$scope.fileFormatsHuman = FileFormats.getFileFormatsHuman() + $translate.instant('import.gz.zip');
Expand Down Expand Up @@ -176,16 +169,8 @@ importViewModule.controller('ImportViewCtrl', ['$scope', 'toastr', '$interval',
});
};

/**
* Get selection from the resource tree.
* @return {string[]}
*/
$scope.getSelectedFiles = () => {
return ImportContextService.getSelectedFilesNames();
};

$scope.importSelected = (overrideSettings) => {
const selected = new Set([...Object.keys($scope.selectedForImportFiles), ...$scope.getSelectedFiles()]);
const selected = new Set([...getImplicitlySelectedFiles(), ...getExplicitlySelectedFiles()]);
const selectedFileNames = Array.from(selected);

// Calls the REST API sequentially for the selected files
Expand Down Expand Up @@ -232,16 +217,6 @@ importViewModule.controller('ImportViewCtrl', ['$scope', 'toastr', '$interval',
loadDefaultSettings();
};

const getDefaultSettings = () => {
if (defaultSettings) {
return Promise.resolve(angular.copy(defaultSettings));
}
return loadDefaultSettings()
.then(() => {
return angular.copy(defaultSettings);
});
};

/**
* Handles the import operation triggered by the user.
* @param {ImportResourceTreeElement} resource - The resource for which the import should be triggered.
Expand Down Expand Up @@ -287,11 +262,6 @@ importViewModule.controller('ImportViewCtrl', ['$scope', 'toastr', '$interval',
* @param {boolean} withoutChangingSettings - Whether to use default settings or not.
*/
$scope.importAll = (selectedResources, withoutChangingSettings) => {
// mark all files as selected locally in order to have them after the import is confirmed via the modal
selectedResources
.forEach((resource) => {
$scope.selectedForImportFiles[resource.path] = true;
});
if (withoutChangingSettings) {
$scope.importSelected(selectedResources, withoutChangingSettings);
} else {
Expand Down Expand Up @@ -343,6 +313,25 @@ importViewModule.controller('ImportViewCtrl', ['$scope', 'toastr', '$interval',
});
};

/**
* Get selection from the resource tree. These are files explicitly selected by the user by checking the
* checkboxes.
* @return {string[]}
*/
const getExplicitlySelectedFiles = () => {
return ImportContextService.getSelectedFilesNames();
};

/**
* Get the names of the files which were just uploaded.
* @return {string[]}
*/
const getImplicitlySelectedFiles = () => {
return $scope.currentFiles
.filter((file) => file.name)
.map((file) => file.name);
};

/**
* Opens a confirmation dialog to confirm the removal of the selected resources. If removal is confirmed, a
* request is sent to the backend with the selected file names to be removed.
Expand All @@ -359,20 +348,31 @@ importViewModule.controller('ImportViewCtrl', ['$scope', 'toastr', '$interval',
});
};

const getDefaultSettings = () => {
if (defaultSettings) {
return Promise.resolve(angular.copy(defaultSettings));
}
return loadDefaultSettings()
.then(() => {
return angular.copy(defaultSettings);
});
};

const getSettingsFor = (fileName, withDefaultSettings) => {
if (!withDefaultSettings && !_.isEmpty(fileName) && !_.isEmpty($scope.savedSettings[fileName])) {
return Promise.resolve($scope.savedSettings[fileName]);
} else {
return getDefaultSettings()
.then((defaultSettings) => {
return defaultSettings
return defaultSettings;
});
}
};

// TODO: temporary exposed in the scope because it is called via scope.parent from the child TabsCtrl which should be changed
/**
* @param {boolean} force - Force the files list to be replaced with the new data
* @return {Promise} A promise which is self resolved. An ugly legacy solution which we didn't want to change now.
*/
$scope.updateListHttp = (force) => {
const filesLoader = $scope.activeTabId === TABS.USER ? ImportRestService.getUploadedFiles : ImportRestService.getServerFiles;
Expand Down Expand Up @@ -467,11 +467,6 @@ importViewModule.controller('ImportViewCtrl', ['$scope', 'toastr', '$interval',
subscriptions.push($scope.$on('repositoryIsSet', $scope.onRepositoryChange));
subscriptions.push($scope.$on('$destroy', () => $interval.cancel(listPollingHandler)));
subscriptions.push(ImportContextService.onActiveTabIdUpdated((newActiveTabId) => onActiveTabChanged(newActiveTabId)));
subscriptions.push(ImportContextService.onSelectedFilesNamesUpdated(() => {
$scope.selectedForImportFiles = ImportContextService.getSelectedFilesNames()
.map((name) => ({[name]: true}))
.reduce((acc, val) => Object.assign(acc, val), {});
}));
$scope.$on('$destroy', removeAllListeners);
};

Expand Down Expand Up @@ -517,8 +512,7 @@ importViewModule.controller('ImportCtrl', ['$scope', 'toastr', '$controller', '$
* @param {boolean} overrideSettings If default settings should be used or not.
*/
$scope.importSelected = (overrideSettings) => {
const selectedFileNames = Object.keys($scope.selectedForImportFiles);
importServerFiles(selectedFileNames, overrideSettings);
importServerFiles(ImportContextService.getSelectedFilesNames(), overrideSettings);
};

$scope.importFile = function (fileName) {
Expand Down Expand Up @@ -862,7 +856,6 @@ importViewModule.controller('UploadCtrl', ['$scope', 'toastr', '$controller', '$
// the list of currentFiles files to be imported.
$scope.currentFiles.forEach((file) => {
$scope.fileChecked[file.name] = true;
$scope.selectedForImportFiles[file.name] = true;
});
// Provide an import rejected callback and do the upload instead.
let fileName = '';
Expand All @@ -872,20 +865,17 @@ importViewModule.controller('UploadCtrl', ['$scope', 'toastr', '$controller', '$
$scope.setSettingsFor(fileName, false, undefined, () => {
$scope.currentFiles.forEach((file) => {
$scope.updateImport(file.name, false, false);
// reset these as we are just uploading here
$scope.selectedForImportFiles = {};
});
});
}
}
};

const deselectAllFiles = () => {
// TODO: This should be removed at some time
Object.keys($scope.fileChecked).forEach((key) => {
$scope.fileChecked[key] = false;
});
// TODO: The above should be removed
$scope.selectedForImportFiles = {};
};

const removeAllListeners = () => {
Expand Down

0 comments on commit 827462b

Please sign in to comment.