Differential D1727
Immediately download archive if already cooked by the vault Authored by anlambert on Jul 11 2019, 4:51 PM.
Details
When a requested archive has already been cooked by the vault, we can immediately offer its download to users without redirecting to the vault UI page. That diff introduces that improvement plus some small changes:
Related T1858
Diff Detail
Event TimelineComment Actions Build is green Comment Actions Build is green Comment Actions Hi @shisus , sorry for the late review but I was in holidays the last weeks and I just get back into my office. Thanks for your contribution so far but this requires some improvements for the feature to work. I added some inline comments regarding what needs to be improved. Please find below a diff that should help you implement the feature properly. diff --git a/swh/web/assets/src/bundles/vault/index.js b/swh/web/assets/src/bundles/vault/index.js index 8393c768..ef18a125 100644 --- a/swh/web/assets/src/bundles/vault/index.js +++ b/swh/web/assets/src/bundles/vault/index.js @@ -10,4 +10,3 @@ import './vault.css'; export * from './vault-ui'; export * from './vault-create-tasks'; -export * from './vault-utils.js'; diff --git a/swh/web/assets/src/bundles/vault/vault-create-tasks.js b/swh/web/assets/src/bundles/vault/vault-create-tasks.js index 5a5f3e1d..d4fbbfe4 100644 --- a/swh/web/assets/src/bundles/vault/vault-create-tasks.js +++ b/swh/web/assets/src/bundles/vault/vault-create-tasks.js @@ -6,47 +6,31 @@ */ import {handleFetchError, csrfPost} from 'utils/functions'; -import {checkUrlHandler} from "./vault-utils"; -function existCookingTask(cookingTask) { - let vaultCookingTasks = JSON.parse(localStorage.getItem('swh-vault-cooking-tasks')); - if (!vaultCookingTasks) { - vaultCookingTasks = []; +function requestArchiveCookingIfNeeded(objectType, objectId) { + let vaultUrl; + if (objectType === 'directory') { + vaultUrl = Urls.api_1_vault_cook_directory(objectId); + } else { + vaultUrl = Urls.api_1_vault_cook_revision(objectId); } - - return vaultCookingTasks.find(val => { - return val.object_type === cookingTask.object_type && - val.object_id === cookingTask.object_id; - }) -} - -export function checkDirectoryArchive(id) { - checkVaultCookingTask('directory', id); + fetch(vaultUrl) + .then(response => response.json()) + .then(data => { + if (data.status !== 'done') { + $(`#vault-cook-${objectType}-modal`).modal('show'); + } else { + window.location = data.fetch_url; + } + }); } -export function checkRevisionArchive(id) { - checkVaultCookingTask('revision', id); +export function requestDirectoryCookingIfNeeded(directoryId) { + requestArchiveCookingIfNeeded('directory', directoryId); } -function checkVaultCookingTask(type, id) { - const cookingTask = { - object_type: type, - object_id: id - }; - - const exists_task = existCookingTask(cookingTask); - - if (!exists_task) { - let vaultUrl = checkUrlHandler[cookingTask.object_type](cookingTask.object_id); - fetch(vaultUrl) - .then(({ body }) => { - if (body.status !== 'done') { - $(`#vault-cook-${cookingTask.object_type}-modal`).modal('show'); - } else { - window.location = Urls.browse_vault(); - } - }); - } +export function requestRevisionCookingIfNeeded(revisionId) { + requestArchiveCookingIfNeeded('revision', revisionId); } function addVaultCookingTask(cookingTask) { diff --git a/swh/web/assets/src/bundles/vault/vault-utils.js b/swh/web/assets/src/bundles/vault/vault-utils.js deleted file mode 100644 index 277de2fd..00000000 --- a/swh/web/assets/src/bundles/vault/vault-utils.js +++ /dev/null @@ -1,5 +0,0 @@ - -export const checkUrlHandler = { - directory: (object_id) => Urls.api_1_vault_cook_directory(object_id), - revision: (object_id) => Urls.api_1_vault_cook_revision_gitfast(object_id), -}; diff --git a/swh/web/templates/includes/vault-create-tasks.html b/swh/web/templates/includes/vault-create-tasks.html index 952489c3..c47d1e00 100644 --- a/swh/web/templates/includes/vault-create-tasks.html +++ b/swh/web/templates/includes/vault-create-tasks.html @@ -16,12 +16,12 @@ See top-level LICENSE file for more information </a> <div class="dropdown-menu"> {% if vault_cooking.directory_context %} - <button class="dropdown-item" type="button" tabindex="-1" onclick="swh.vault.checkDirectoryArchive('{{ vault_cooking.directory_id }}')"> + <button class="dropdown-item" type="button" tabindex="-1" onclick="swh.vault.requestDirectoryCookingIfNeeded('{{ vault_cooking.directory_id }}')"> <i class="{{ swh_object_icons.directory }} fa-fw" aria-hidden="true"></i>Directory </button> {% endif %} {% if vault_cooking.revision_context %} - <button class="dropdown-item" type="button" tabindex="-1" onclick="swh.vault.checkRevisionArchive('{{ vault_cooking.revision_id }}')"> + <button class="dropdown-item" type="button" tabindex="-1" onclick="swh.vault.requestRevisionCookingIfNeeded('{{ vault_cooking.revision_id }}')"> <i class="{{ swh_object_icons.revision }} fa-fw"></i>Revision </button> {% endif %} I also noticed that webpack did not correctly report linting errors for js and css files, I fixed it today. I invite you to rebase your feature branch on origin/master before integrating the changes detailed above. Next step: Add a test for that new feature using the cypress tool. This will require mocking some calls to the vault API, I need to think on how to do that first.
Comment Actions Thanks for your insights @anlambert.
Comment Actions @shisus, do not forget to rebase to origin/master before submitting an update to that Diff. Once you have integrated your changes, you can update that diff using arcanist the following way: $ arc diff --update D1727
Comment Actions Commandeering this as the diff has not been updated by its original author since two months. Comment Actions Build is green Comment Actions Build has FAILED Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/338/ Comment Actions Build is green Comment Actions Build has FAILED Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/cypress-diff/339/ Comment Actions Build is green Comment Actions Build is green Comment Actions Build is green Comment Actions Build is green | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||