diff --git a/Gemfile.lock b/Gemfile.lock index 1e5ea7a515..f66521c49f 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -362,7 +362,7 @@ GEM psych (5.1.2) stringio public_suffix (5.0.4) - puma (5.6.8) + puma (5.6.9) nio4r (~> 2.0) racc (1.8.0) rack (2.2.9) @@ -520,7 +520,7 @@ GEM zeitwerk (~> 2.2) warden (1.2.9) rack (>= 2.0.9) - webrick (1.8.1) + webrick (1.8.2) websocket-driver (0.7.6) websocket-extensions (>= 0.1.0) websocket-extensions (0.1.5) diff --git a/app/assets/images/cellarium.png b/app/assets/images/cellarium.png new file mode 100644 index 0000000000..01c96b45e1 Binary files /dev/null and b/app/assets/images/cellarium.png differ diff --git a/app/controllers/site_controller.rb b/app/controllers/site_controller.rb index 230e0d9da7..84428bc6d8 100644 --- a/app/controllers/site_controller.rb +++ b/app/controllers/site_controller.rb @@ -61,6 +61,7 @@ def index @cell_count = 0 end + @home_page_link = HomePageLink.published end def covid diff --git a/app/javascript/components/explore/plot-data-cache.js b/app/javascript/components/explore/plot-data-cache.js index 78ac277ea5..abdd92178a 100644 --- a/app/javascript/components/explore/plot-data-cache.js +++ b/app/javascript/components/explore/plot-data-cache.js @@ -289,7 +289,8 @@ export function createCache() { Fields.cellsAndCoords.merge(cacheEntry, scatter) // only merge in annotation values if the annotation matches (or the default was requested, so // we can then assume the response matches) - if (!requestedAnnotation.name || scatter.annotParams.name === requestedAnnotation.name) { + // annotParams may be undefined in spatial UX if a cluster-based annotation does not exist for the plot + if (!requestedAnnotation.name || scatter.annotParams?.name === requestedAnnotation.name) { Fields.annotation.merge(cacheEntry, scatter) } if (scatter.genes.length && scatter.genes.join('') === requestedGenes.join('')) { diff --git a/app/javascript/components/upload/FileUploadControl.jsx b/app/javascript/components/upload/FileUploadControl.jsx index be806d26cc..6a26dbf902 100644 --- a/app/javascript/components/upload/FileUploadControl.jsx +++ b/app/javascript/components/upload/FileUploadControl.jsx @@ -28,6 +28,10 @@ export default function FileUploadControl({ const [showUploadButton, setShowUploadButton] = useState(true) const [showBucketPath, setShowBucketPath] = useState(false) const ToggleUploadButton = () => { + // this is an inverted check since the user is clicking and the value is about to change + if (!showUploadButton) { + unsetRemoteLocation() + } setShowUploadButton(!showUploadButton) setShowBucketPath(!showBucketPath) } @@ -38,7 +42,7 @@ export default function FileUploadControl({ 'Upload a file from your computer' : "Input a path to a file that is already in this study's bucket" const uploadToggle = {toggleText} @@ -51,7 +55,7 @@ export default function FileUploadControl({ const googleBucketLink = - Browse bucket @@ -77,20 +81,53 @@ export default function FileUploadControl({ name: newName, notes }) + } else if (issues.errors.length > 0 && file.uploadSelection) { + // clear out a previous known good file, if present + updateFile(file._id, { + uploadSelection: null, + upload_file_name: '', + name: '' + }) } } + // keep track of pending timeout for remote validation via bucket path + const [timeOutId, setTimeOutID] = useState(null) + + // clear out remote_location and hasRemoteFile to allow switching back to file upload button + function unsetRemoteLocation() { + updateFile(file._id, {remote_location: '', hasRemoteFile: false}) + } + // perform CSFV on remote file when specifying a GS URL or bucket path // will sanitize GS URL before calling validateRemoteFile - async function handleBucketLocationEntry(e) { - const path = e.target.value + async function handleBucketLocationEntry(path) { const matcher = new RegExp(`(gs:\/\/)?${bucketName}\/?`) const trimmedPath = path.replace(matcher, '') if (!trimmedPath) { + unsetRemoteLocation() + setFileValidation({ validating: false, issues: {}, fileName: null }) return false } + // don't continue unless a dot is present (otherwise, no valid file extension) + if (!trimmedPath.includes('.')) { return false } + const fileType = file.file_type + const fileExtension = `.${trimmedPath.split('.').slice(-1)[0]}` + if (fileExtension.length > 1 && !inputAcceptExts.includes(fileExtension)) { + const invalidExt = { + errors: [ + [ + 'error', 'filename:extension', + `Allowed extensions are ${allowedFileExts.join(', ')}` + ] + ] + } + setFileValidation({ validating: false, issues: invalidExt, fileName: trimmedPath }) + return false + } + const fileOptions = fileType === 'Metadata' ? { use_metadata_convention: file?.use_metadata_convention } : {} setFileValidation({ validating: true, issues: {}, fileName: trimmedPath }) @@ -175,6 +212,7 @@ export default function FileUploadControl({ /> } + {!isFileOnServer && (showBucketPath || file.hasRemoteFile ) && // we can't use TextFormField since we need a custom onBlur event // onBlur is the React equivalent of onfocusout, which will fire after the user is done updating the input @@ -182,15 +220,22 @@ export default function FileUploadControl({ type="text" size={60} id={`remote_location-input-${file._id}`} + data-testid="remote-location-input" placeholder='GS URL or path to file in GCP bucket' - onBlur={handleBucketLocationEntry}/> + onChange={ (e) => { + const newBucketPath = e.target.value + if (timeOutId) { clearTimeout(timeOutId) } + const newTimeout = setTimeout(handleBucketLocationEntry, 300, newBucketPath) + setTimeOutID(newTimeout) + }}/> } -    { !isFileOnServer && (showBucketPath || file.hasRemoteFile ) && googleBucketLink } -    { !isFileOnServer && uploadToggle } + { showBucketPath && fileValidation.validating && + Validating... + } { setIsAnnDataExperience(true) setOverrideExperienceMode(true) - }}> AnnData BETA + }}> AnnData
Upload one AnnData (.h5ad) file diff --git a/app/javascript/components/upload/UploadWizard.jsx b/app/javascript/components/upload/UploadWizard.jsx index 4da9cc59ef..18b9343665 100644 --- a/app/javascript/components/upload/UploadWizard.jsx +++ b/app/javascript/components/upload/UploadWizard.jsx @@ -275,12 +275,17 @@ export function RawUploadWizard({ studyAccession, name }) { setTimeout(() => deleteFileFromServer(requestCanceller.fileId), 500) } + /** helper for determining when to use saveAnnDataFileHelper (sets ids/values correctly for AnnData UX **/ + function useAnnDataFileHelper(file) { + return isAnnDataExperience && (file?.file_type === 'AnnData' || Object.keys(file).includes("data_type")) + } + /** save the given file and perform an upload if a selected file is present */ async function saveFile(file) { let fileToSave = file let studyFileId = file._id - if (isAnnDataExperience && fileToSave?.file_type === 'AnnData') { + if (useAnnDataFileHelper(fileToSave)) { fileToSave = saveAnnDataFileHelper(file, fileToSave) studyFileId = fileToSave._id } diff --git a/app/javascript/components/upload/WizardNavPanel.jsx b/app/javascript/components/upload/WizardNavPanel.jsx index 44123b1d28..b7e9558520 100644 --- a/app/javascript/components/upload/WizardNavPanel.jsx +++ b/app/javascript/components/upload/WizardNavPanel.jsx @@ -222,7 +222,7 @@ function MainStepsDisplay(formState, serverState, currentStep, setCurrentStep, m - AnnData BETA + AnnData diff --git a/app/javascript/components/visualization/ScatterPlot.jsx b/app/javascript/components/visualization/ScatterPlot.jsx index 0062c3949e..4cca8f7494 100644 --- a/app/javascript/components/visualization/ScatterPlot.jsx +++ b/app/javascript/components/visualization/ScatterPlot.jsx @@ -598,17 +598,15 @@ function RawScatterPlot({ return (
{ ErrorComponent } + { hasMissingAnnot &&
"{cluster}" does not have the requested annotation "{loadedAnnotation}"
} - { !hasMissingAnnot && - - }
{ + if (name.startsWith('ontologies-') && name !== currentOntologies) { + caches.delete(name) + } + }) + + const cache = await caches.open(currentOntologies) + + return cache +} + +/** Fetch .gz file, decompress it, return plaintext */ +export async function fetchGzipped(url) { + const response = await fetch(url) + const blob = await response.blob() + const uint8Array = new Uint8Array(await blob.arrayBuffer()) + const plaintext = strFromU8(decompressSync(uint8Array)) + return plaintext +} + +/** Fetch from service worker cache if available, from remote otherwise */ +export async function cacheFetch(url) { + const cache = await getServiceWorkerCache() + + const decompressedUrl = url.replace('.gz', '') + const response = await cache.match(decompressedUrl) + if (typeof response === 'undefined') { + // If cache miss, then fetch, decompress, and put response in cache + const data = await fetchGzipped(url) + const contentLength = data.length + const decompressedResponse = new Response( + data, + { + headers: new Headers({ + 'Content-Length': contentLength, + 'Content-Type': 'text/tab-separated-values' + }) + } + ) + await cache.put(decompressedUrl, decompressedResponse) + return await cache.match(decompressedUrl) + } + return await cache.match(decompressedUrl) +} + +/** + * Fetch minified ontologies, transform into object of object of arrays, e.g.: + * + * { + * 'mondo': { + * 'MONDO_0008315': ['prostate cancer', 'prostate neoplasm', 'prostatic neoplasm'], + * 'MONDO_0018076': ['tuberculosis', 'TB'], + * ... + * }, + * 'ncbitaxon': { + * 'NCBITaxon_9606': ['Homo sapiens', 'human'], + * 'NCBITaxon_10090': ['Mus musculus', 'house mouse', 'mouse'], + * ... + * }, + * ... + * } + */ +export async function fetchOntologies() { + if (window.SCP.ontologies) { + // Reuse fetched, processed ontologies from this page load + return window.SCP.ontologies + } + + const ontologies = {} + + const ontologyNames = getOntologyShortNames() + + for (let i = 0; i < ontologyNames.length; i++) { + const ontologyName = ontologyNames[i] + const ontologyUrl = `${ONTOLOGY_BASE_URL + ontologyName}.min.tsv.gz` + const response = await cacheFetch(ontologyUrl) + + const tsv = await response.text() + const lines = tsv.split('\n') + + ontologies[ontologyName] = {} + + for (let i = 0; i < lines.length; i++) { + const line = lines[i] + const [ontologyId, label, rawSynonyms] = line.split('\t') + let names = [label] + if (rawSynonyms) { + const synonyms = rawSynonyms.split('||') + names = names.concat(synonyms) + } + ontologies[ontologyName][ontologyId] = names + } + } + + window.SCP.ontologies = ontologies + return ontologies +} + +/** Get lowercase shortnames for all required ontologies */ +function getOntologyShortNames() { + let requiredOntologies = [] + + // Validate IDs for species, organ, disease, and library preparation protocol + for (let i = 0; i < REQUIRED_CONVENTION_COLUMNS.length; i++) { + const column = REQUIRED_CONVENTION_COLUMNS[i] + if (!column.endsWith('__ontology_label')) {continue} + const key = column.split('__ontology_label')[0] + const ontologies = getAcceptedOntologies(key, metadataSchema) + requiredOntologies = requiredOntologies.concat(ontologies) + } + + requiredOntologies = Array.from( + new Set(requiredOntologies.map(o => o.toLowerCase())) + ) + + return requiredOntologies +} + +/** + * Get list of ontology names accepted for key from metadata schema + * + * E.g. "disease" -> ["MONDO", "PATO"] + */ +export function getAcceptedOntologies(key, metadataSchema) { + // E.g. "ontology_browser_url": "https://www.ebi.ac.uk/ols/ontologies/mondo,https://www.ebi.ac.uk/ols/ontologies/pato" + const olsUrls = metadataSchema.properties[key].ontology + + const acceptedOntologies = + olsUrls?.split(',').map(url => url.split('/').slice(-1)[0].toUpperCase()) + + if (acceptedOntologies.includes('NCBITAXON')) { + acceptedOntologies.push('NCBITaxon') + } + + return acceptedOntologies +} diff --git a/app/javascript/lib/validation/validate-anndata.js b/app/javascript/lib/validation/validate-anndata.js index d05fbe7bd9..7e5f4fc497 100644 --- a/app/javascript/lib/validation/validate-anndata.js +++ b/app/javascript/lib/validation/validate-anndata.js @@ -1,10 +1,11 @@ import {openH5File} from 'hdf5-indexed-reader' +import { getOAuthToken } from '~/lib/scp-api' import { validateUnique, validateRequiredMetadataColumns, metadataSchema, REQUIRED_CONVENTION_COLUMNS } from './shared-validation' -import { getOAuthToken } from '~/lib/scp-api' +import { getAcceptedOntologies, fetchOntologies } from './ontology-validation' /** Get ontology ID values for key in AnnData file */ async function getOntologyIds(key, hdf5File) { @@ -90,25 +91,6 @@ export async function getAnnDataHeaders(hdf5File) { return headers } -/** - * Get list of ontology names accepted for key from metadata schema - * - * E.g. "disease" -> ["MONDO", "PATO"] - */ -function getAcceptedOntologies(key, metadataSchema) { - // E.g. "ontology_browser_url": "https://www.ebi.ac.uk/ols/ontologies/mondo,https://www.ebi.ac.uk/ols/ontologies/pato" - const olsUrls = metadataSchema.properties[key].ontology - - const acceptedOntologies = - olsUrls?.split(',').map(url => url.split('/').slice(-1)[0].toUpperCase()) - - if (acceptedOntologies.includes('NCBITAXON')) { - acceptedOntologies.push('NCBITaxon') - } - - return acceptedOntologies -} - /** * Check format of ontology IDs for key, return updated issues array * @@ -138,6 +120,127 @@ export function checkOntologyIdFormat(key, ontologyIds) { return issues } +/** Validate author's annotation labels and IDs match those in ontologies */ +async function checkOntologyLabelsAndIds(key, ontologies, groups) { + const [ids, idIndexes, labels, labelIndexes] = groups + + const issues = [] + + // Determine unique (ontology ID, ontology label) pairs + const labelIdPairs = new Set() + for (let i = 0; i < idIndexes.length; i++) { + const id = ids[idIndexes[i]] + const label = labels[labelIndexes[i]] + labelIdPairs.add(`${id} || ${label}`) + } + const rawUniques = Array.from(labelIdPairs) + + rawUniques.map(r => { + const [id, label] = r.split(' || ') + const ontologyShortNameLc = id.split(/[_:]/)[0].toLowerCase() + const ontology = ontologies[ontologyShortNameLc] + + if (!(id in ontology)) { + // Register invalid ontology ID + const msg = `Invalid ontology ID: ${id}` + issues.push([ + 'error', 'ontology:label-lookup-error', msg, + { subtype: 'ontology:invalid-id' } + ]) + } else { + const validLabels = ontology[id] + + if (!(validLabels.includes(label))) { + // Register invalid ontology label + const prettyLabels = validLabels.join(', ') + const validLabelsClause = `Valid labels for ${id}: ${prettyLabels}` + const msg = `Invalid ${key} label "${label}". ${validLabelsClause}` + issues.push([ + 'error', 'ontology:label-lookup-error', msg, + { subtype: 'ontology:invalid-label' } + ]) + } + } + }) + + return issues +} + +/** Get ontology ID values for key in AnnData file */ +async function getOntologyIdsAndLabels(requiredName, hdf5File) { + const obs = await hdf5File.get('obs') + const obsValues = await Promise.all(obs.values) + + // Old versions of the AnnData spec used __categories as an obs. + // However, in new versions (since before 2023-01-23) of AnnData spec, + // categorical arrays are encoded as self-contained groups containing their + // own `categories` and `codes`. + // See e.g. https://github.com/scverse/anndata/issues/879 + const internalCategories = obsValues.find(o => o.name.endsWith('__categories')) + if (internalCategories) { + console.debug( + 'Encountered old-spec AnnData, skipping ontology label validation. ' + + 'Server-side processing will validate this' + ) + return null + } + + const idKey = requiredName + const labelKey = `${requiredName}__ontology_label` + + const idGroup = obsValues.find(o => o.name.endsWith(idKey)) + const labelGroup = obsValues.find(o => o.name.endsWith(labelKey)) + + // AnnData organizes each "obs" annotation (e.g. disease__ontology_label, + // sex) into a container with a `categories` frame and a `code` frame. + // + // - categories: external values, non-redundant array. E.g.: + // ["tuberculosis", "TB", "foo"] or ["female"] + // + // - codes: internal values, redundant array of integers that specify + // the index (position) of each category value in the array of obs + // (cells) + // + // This organization greatly decreases filesize, but requires more code + // to map paired obs annotations like `disease` (ontology IDs) to + // `disease__ontology_label` (ontology names) than needed for e.g. TSVs. + const idCategories = await idGroup.values[0] + const idCodes = await idGroup.values[1] + const ids = await idCategories.value + const idIndexes = await idCodes.value + + const labelCategories = await labelGroup.values[0] + const labelCodes = await labelGroup.values[1] + const labels = await labelCategories.value + const labelIndexes = await labelCodes.value + + return [ids, idIndexes, labels, labelIndexes] +} + +/** Validate ontology labels for required metadata columns in AnnData file */ +async function validateOntologyLabelsAndIds(hdf5File) { + let issues = [] + + const ontologies = await fetchOntologies() + + // Validate IDs for species, organ, disease, and library preparation protocol + for (let i = 0; i < REQUIRED_CONVENTION_COLUMNS.length; i++) { + const column = REQUIRED_CONVENTION_COLUMNS[i] + if (!column.endsWith('__ontology_label')) {continue} + const key = column.split('__ontology_label')[0] + const groups = await getOntologyIdsAndLabels(key, hdf5File) + + if (groups) { + issues = issues.concat( + await checkOntologyLabelsAndIds(key, ontologies, groups) + ) + } + } + + return issues +} + + /** Validate ontology IDs for required metadata columns in AnnData file */ async function validateOntologyIdFormat(hdf5File) { let issues = [] @@ -165,21 +268,26 @@ export async function parseAnnDataFile(fileOrUrl, remoteProps) { const headers = await getAnnDataHeaders(hdf5File) - // TODO (SCP-5770): Extend AnnData CSFV to remote files, then remove this - if (!headers) { - return { issues } - } - const requiredMetadataIssues = validateRequiredMetadataColumns([headers], true) let ontologyIdFormatIssues = [] + let ontologyLabelAndIdIssues = [] if (requiredMetadataIssues.length === 0) { ontologyIdFormatIssues = await validateOntologyIdFormat(hdf5File) + if ( + ontologyIdFormatIssues.length === 0 && + + // TODO (SCP-5813): Enable ontology validation for remote AnnData + remoteProps && 'url' in remoteProps === false + ) { + ontologyLabelAndIdIssues = await validateOntologyLabelsAndIds(hdf5File) + } } issues = issues.concat( validateUnique(headers), requiredMetadataIssues, - ontologyIdFormatIssues + ontologyIdFormatIssues, + ontologyLabelAndIdIssues ) return { issues } diff --git a/app/javascript/styles/_brand.scss b/app/javascript/styles/_brand.scss index 69623bdad2..097928b994 100644 --- a/app/javascript/styles/_brand.scss +++ b/app/javascript/styles/_brand.scss @@ -252,11 +252,17 @@ } } -#whitepaper-btn { +#home-page-link { position: absolute; top: 220px; right: 15px; z-index: 500; + + img { + height: 24px; + object-fit: scale-down; + padding-right: 5px; + } } #latest-features-btn { diff --git a/app/javascript/styles/_global.scss b/app/javascript/styles/_global.scss index 1616a0dc19..2bd1cded46 100644 --- a/app/javascript/styles/_global.scss +++ b/app/javascript/styles/_global.scss @@ -941,6 +941,16 @@ figcaption.ck-editor__editable { color: #C45500; } +.btn-home-link { + color: #fff; + box-shadow: 8px 8px 12px #2b2b2b; +} + +.btn-home-link:hover { + color: #fff; + filter: brightness(90%); +} + .link-darker-blue { color: #1b476d } diff --git a/app/lib/download_quota_service.rb b/app/lib/download_quota_service.rb index 6877d28afd..60f014b955 100644 --- a/app/lib/download_quota_service.rb +++ b/app/lib/download_quota_service.rb @@ -19,7 +19,7 @@ def self.grant_user_exemption(user) def self.download_exceeds_quota?(user, requested_bytes) return false if user.daily_download_quota.nil? - user_quota = user.daily_download_quota + requested_bytes + user_quota = user.daily_download_quota + requested_bytes.to_i user_quota > download_quota end @@ -28,7 +28,7 @@ def self.increment_user_quota(user, requested_bytes) # skip incrementing quota if exemption is set via nil value return true if user.daily_download_quota.nil? - user_quota = user.daily_download_quota + requested_bytes + user_quota = user.daily_download_quota + requested_bytes.to_i user.update(daily_download_quota: user_quota) end diff --git a/app/mailers/single_cell_mailer.rb b/app/mailers/single_cell_mailer.rb index ddbf128657..61c8bbebcb 100644 --- a/app/mailers/single_cell_mailer.rb +++ b/app/mailers/single_cell_mailer.rb @@ -38,7 +38,7 @@ def notify_admin_upload_fail(study_file, error) @study_file = study_file @error = error @user = @study.user - mail(to: emails, subject: '[Single Cell Portal ERROR] FireCloud auto-upload fail in ' + @study.accession) do |format| + mail(to: emails, subject: "[Single Cell Portal ERROR] FireCloud auto-upload fail in #{@study.accession}") do |format| format.html end end @@ -51,19 +51,19 @@ def notify_user_upload_fail(study_file, study, user) dev_email_config = AdminConfiguration.find_by(config_type: 'QA Dev Email') dev_email = dev_email_config.present? ? dev_email_config.value : nil title = "#{study_file.upload_file_name} did not finish uploading" - mail(to: user.email, bcc: dev_email, subject: '[Single Cell Portal Notifier] ' + title) + mail(to: user.email, bcc: dev_email, subject: "[Single Cell Portal Notifier] #{title}") end def notify_user_parse_complete(email, title, message, study) @message = message @study = study - mail(to: email, subject: '[Single Cell Portal Notifier] ' + title) + mail(to: email, subject: "[Single Cell Portal Notifier] #{title}") end def notify_user_parse_fail(email, title, error, study) @error = error @study = study - mail(to: email, subject: '[Single Cell Portal Notifier] ' + title) + mail(to: email, subject: "[Single Cell Portal Notifier] #{title}") end def notify_admin_parse_fail(user_email, title, contents) @@ -72,7 +72,7 @@ def notify_admin_parse_fail(user_email, title, contents) dev_email = dev_email_config.value @contents = contents @user_email = user_email - mail(to: dev_email, subject: '[Single Cell Portal Admin Notification] ' + title) + mail(to: dev_email, subject: "[Single Cell Portal Admin Notification] #{title}") end end @@ -91,7 +91,7 @@ def notify_admin_parse_launch_fail(study, study_file, corresponding_user, ingest @error = error @action = ingest_action title = 'Ingest Pipeline Launch Failure' - mail(to: emails, subject: '[Single Cell Portal Admin Notification] ' + title) + mail(to: emails, subject: "[Single Cell Portal Admin Notification] #{title}") end def daily_disk_status diff --git a/app/models/ann_data_file_info.rb b/app/models/ann_data_file_info.rb index 3122281736..a0ce6f4d5b 100644 --- a/app/models/ann_data_file_info.rb +++ b/app/models/ann_data_file_info.rb @@ -39,7 +39,7 @@ class AnnDataFileInfo # } # { _id: '6033f531e241391884633748', data_type: :expression, description: 'log(TMP) expression' } field :data_fragments, type: Array, default: [] - before_validation :sanitize_fragments! + before_validation :set_default_cluster_fragments!, :sanitize_fragments! validate :validate_fragments after_validation :update_expression_file_info @@ -195,6 +195,20 @@ def sanitize_fragments! self.data_fragments = sanitized_fragments end + # create the default cluster data_fragment entries + def set_default_cluster_fragments! + return false if fragments_by_type(:cluster).any? + + default_obsm_keys = AnnDataIngestParameters::PARAM_DEFAULTS[:obsm_keys] + default_obsm_keys.each do |obsm_key_name| + name = obsm_key_name.delete_prefix('X_') + fragment = { + _id: BSON::ObjectId.new.to_s, data_type: :cluster, name:, obsm_key_name:, spatial_cluster_associations: [] + } + data_fragments << fragment + end + end + # ensure all fragments have required keys and are unique def validate_fragments REQUIRED_FRAGMENT_KEYS.each do |data_type, keys| diff --git a/app/models/ann_data_ingest_parameters.rb b/app/models/ann_data_ingest_parameters.rb index 82729cd914..be3f26fd8b 100644 --- a/app/models/ann_data_ingest_parameters.rb +++ b/app/models/ann_data_ingest_parameters.rb @@ -32,7 +32,7 @@ class AnnDataIngestParameters PARAM_DEFAULTS = { ingest_anndata: true, anndata_file: nil, - obsm_keys: %w[X_umap X_tsne], + obsm_keys: %w[X_umap], ingest_cluster: false, cluster_file: nil, name: nil, diff --git a/app/models/home_page_link.rb b/app/models/home_page_link.rb new file mode 100644 index 0000000000..b28b3ade30 --- /dev/null +++ b/app/models/home_page_link.rb @@ -0,0 +1,78 @@ +class HomePageLink + include Mongoid::Document + include Mongoid::Timestamps + + DEFAULT_CSS_CLASS='btn btn-home-link'.freeze + DEFAULT_BG_COLOR='#4999F9'.freeze + + field :name, type: String + field :href, type: String + field :tooltip, type: String + field :bg_color, type: String, default: DEFAULT_BG_COLOR + field :css_class, type: String, default: DEFAULT_CSS_CLASS + field :published, type: Mongoid::Boolean, default: false + field :image, type: String + + validates :name, :href, presence: true + validate :ensure_one_published_link + + def publish! + update(published: true) + end + + def unpublish! + update(published: false) + end + + def reset_css! + puts "Resetting css_class to '#{DEFAULT_CSS_CLASS}'" + update(css_class: DEFAULT_CSS_CLASS) + end + + def reset_bg_color! + puts "Resetting bg_color to '#{DEFAULT_BG_COLOR}'" + update(bg_color: DEFAULT_BG_COLOR) + end + + def self.published + self.find_by(published: true) + end + + def self.publish_last! + if published + puts "'#{published.name}' is already published" + return false + end + + link = last + if link + puts "Publishing '#{link.name}'" + link.publish! + else + puts "Nothing to publish" + end + end + + def self.unpublish! + if published.present? + puts "Unpublishing '#{published.name}'" + published.update(published: false) + else + puts "No published links" + return false + end + end + + private + + def ensure_one_published_link + if published && HomePageLink.where(published: true, :id.ne => self.id).exists? + existing = HomePageLink.published + errors.add( + :published, + "link exists: '#{existing.name}', please unpublish first with HomePageLink.unpublish!" + ) + puts errors.full_messages.to_sentence + end + end +end diff --git a/app/models/import_service_config/nemo.rb b/app/models/import_service_config/nemo.rb index c6f5f1f829..97d4e7a4ba 100644 --- a/app/models/import_service_config/nemo.rb +++ b/app/models/import_service_config/nemo.rb @@ -72,7 +72,7 @@ def study_file_default_settings # retrieve common species names from associated collection def taxon_names - load_study&.[]('taxonomies') || [] + load_study&.[]('taxa')&.map { |t| t['name']} || [] end # map attribute names SCP Study attributes onto NeMO attribute names diff --git a/app/models/ingest_job.rb b/app/models/ingest_job.rb index 6d8869b275..16f6d15924 100644 --- a/app/models/ingest_job.rb +++ b/app/models/ingest_job.rb @@ -377,7 +377,8 @@ def poll_for_completion(run_at: 1.minute.from_now) log_error_messages log_to_mixpanel # log before queuing file for deletion to preserve properties # don't delete files or notify users if this is a 'special action', like DE or image pipeline jobs - handle_ingest_failure unless special_action? + subject = "Error: #{study_file.file_type} file: '#{study_file.upload_file_name}' parse has failed" + handle_ingest_failure(subject) unless special_action? admin_email_content = generate_error_email_body(email_type: :dev) SingleCellMailer.notify_admin_parse_fail(user.email, subject, admin_email_content).deliver_now else @@ -390,7 +391,7 @@ def poll_for_completion(run_at: 1.minute.from_now) # will automatically clean up data and notify user # in case of subsampling, only subsampled data cleanup is run and all other data is left in place # this reduces churn for study owners as full-resolution data is still valid - def handle_ingest_failure + def handle_ingest_failure(email_subject) if action.to_sym == :ingest_subsample study_file.update(parse_status: 'parsed') # reset parse flag cluster_name = cluster_name_by_file_type @@ -408,9 +409,8 @@ def handle_ingest_failure end end end - subject = "Error: #{study_file.file_type} file: '#{study_file.upload_file_name}' parse has failed" user_email_content = generate_error_email_body - SingleCellMailer.notify_user_parse_fail(user.email, subject, user_email_content, study).deliver_now + SingleCellMailer.notify_user_parse_fail(user.email, email_subject, user_email_content, study).deliver_now end # TODO (SCP-4709, SCP-4710) Processed and Raw expression files diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index dffd0e8167..1f462e1a2f 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -192,10 +192,12 @@ © <%= Date.today.year %> The Broad Institute of MIT and Harvard
- <%= scp_link_to 'Privacy Policy', privacy_policy_path %> -   - <%= scp_link_to "Terms of Service".html_safe, terms_of_service_path %> -   + <%= scp_link_to 'Privacy Policy', privacy_policy_path %>  + <%= scp_link_to 'Terms of Service', terms_of_service_path %>  + <%= scp_link_to 'Portal whitepaper', "https://www.biorxiv.org/content/10.1101/2023.07.13.548886v1", + id: 'whitepaper-btn', data: { toggle: 'tooltip', placement: 'top' }, + title: 'Read the Single Cell Portal whitepaper!', target: :_blank %> + <% if @selected_branding_group.present? %> <%= link_to " Return to Single Cell Portal".html_safe, site_path %> <% end %> diff --git a/app/views/site/_home_page_link.html.erb b/app/views/site/_home_page_link.html.erb new file mode 100644 index 0000000000..d072785842 --- /dev/null +++ b/app/views/site/_home_page_link.html.erb @@ -0,0 +1,11 @@ +<% image_link = @home_page_link.image.present? ? "#{image_tag(@home_page_link.image)}" : nil %> + +<%= + link_to "#{image_link}#{@home_page_link.name}".html_safe, @home_page_link.href, title: @home_page_link.tooltip, + data: { toggle: 'tooltip', placement: 'left'}, class: @home_page_link.css_class, + id: 'home-page-link', target: :_blank +%> diff --git a/app/views/site/_main_banner_content.html.erb b/app/views/site/_main_banner_content.html.erb index 75b016e85f..0cbd745e3c 100644 --- a/app/views/site/_main_banner_content.html.erb +++ b/app/views/site/_main_banner_content.html.erb @@ -5,10 +5,9 @@ latest_feature_announcements_path, class: 'btn btn-warning-scp', id: 'latest-features-btn', data: { toggle: 'tooltip', placement: 'left' }, title: 'Learn about new Single Cell Portal features!' %> <% end %> - <%= link_to " " \ - "Portal whitepaper".html_safe, - "https://www.biorxiv.org/content/10.1101/2023.07.13.548886v1", class: 'btn btn-warning-scp', id: 'whitepaper-btn', - data: { toggle: 'tooltip', placement: 'left' }, title: 'Read the Single Cell Portal whitepaper!', target: :_blank %> + <% if @home_page_link.present? %> + <%= render partial: 'home_page_link' %> + <% end %> <% if @selected_branding_group.present? %> <% content_for(:html_title) { "#{@selected_branding_group.name} - Collections - Single Cell Portal" } %> <% if @selected_branding_group.banner_image.file %> diff --git a/db/seed/synthetic_studies/chicken_raw_counts/study_info.json b/db/seed/synthetic_studies/chicken_raw_counts/study_info.json index 4d51a06b98..228d16593c 100644 --- a/db/seed/synthetic_studies/chicken_raw_counts/study_info.json +++ b/db/seed/synthetic_studies/chicken_raw_counts/study_info.json @@ -39,6 +39,11 @@ "is_spatial": true, "spatial_cluster_associations": ["X cluster"] }, + { + "type": "Cluster", + "filename": "cluster_odd_labels.txt", + "name": "cluster_odd_labels" + }, { "type": "Cluster", "filename": "cluster_square.txt", diff --git a/package.json b/package.json index a86bdf0ddd..63230c6f9f 100644 --- a/package.json +++ b/package.json @@ -106,7 +106,7 @@ "prop-types": "^15.7.2", "react-select-event": "^5.3.0", "stylelint-prettier": "^1.1.2", - "vite": "^4.0.0", + "vite": "^4.5.5", "vite-plugin-ruby": "^3.0.5" } } diff --git a/test/controllers/site_controller_test.rb b/test/controllers/site_controller_test.rb index 0e0f0872b2..3b5b9b4ff8 100644 --- a/test/controllers/site_controller_test.rb +++ b/test/controllers/site_controller_test.rb @@ -249,4 +249,13 @@ def teardown assert @study.publications.empty? end end + + test 'should show home page link' do + get site_path + assert_select '#home-page-link', 0 + HomePageLink.create(name: 'Foo', href: 'http://foo.com', published: true) + get site_path + assert_select '#home-page-link', 1 + HomePageLink.delete_all + end end diff --git a/test/factories/study_file.rb b/test/factories/study_file.rb index f6559f8db2..a6ca5f08bf 100644 --- a/test/factories/study_file.rb +++ b/test/factories/study_file.rb @@ -173,6 +173,7 @@ # phex: [['cellA', 0.6],['cellB', 6.1], ['cellC', 4.5]] # } expression_input { {} } + has_raw_counts { true } end after(:create) do |file, evaluator| file.build_ann_data_file_info @@ -195,7 +196,7 @@ file.build_expression_file_info(library_preparation_protocol: "10x 5' v3") file.expression_file_info.save file.ann_data_file_info.has_expression = true - file.ann_data_file_info.has_raw_counts = true + file.ann_data_file_info.has_raw_counts = evaluator.has_raw_counts %w[raw processed].each do |matrix_type| FactoryBot.create(:data_array, array_type: 'cells', diff --git a/test/integration/external/import_service_config/nemo_test.rb b/test/integration/external/import_service_config/nemo_test.rb index 26de92b663..13abe6d23b 100644 --- a/test/integration/external/import_service_config/nemo_test.rb +++ b/test/integration/external/import_service_config/nemo_test.rb @@ -96,8 +96,8 @@ class NemoTest < ActiveSupport::TestCase test 'should load study analog' do study = @configuration.load_study assert_equal '"Human variation study (10x), GRU"', study['name'] - assert_equal "10x chromium 3' v3 sequencing", study['technique'] - assert_equal %w[human], study['taxonomies'] + assert_equal ["10x chromium 3' v3 sequencing"], study['techniques'] + assert_equal [{"name"=>"human", "cv_term_id"=>"NCBI:txid9606"}], study['taxa'] end # TODO: SCP-5565 Check with NeMO re API, update and re-enable this test diff --git a/test/js/lib/node-web-api.js b/test/js/lib/node-web-api.js new file mode 100644 index 0000000000..6c8ef04d28 --- /dev/null +++ b/test/js/lib/node-web-api.js @@ -0,0 +1,169 @@ +/* eslint-disable require-jsdoc */ +/** @fileoverview Node port for interfaces of Web API; helps mocking + * + * https://developer.mozilla.org/en-US/docs/Web/API/CacheStorage + * https://developer.mozilla.org/en-US/docs/Web/API/Cache + * https://developer.mozilla.org/en-US/docs/Web/API/Request + * https://developer.mozilla.org/en-US/docs/Web/API/Response + * https://developer.mozilla.org/en-US/docs/Web/API/Headers +*/ +const { Readable } = require('stream') + +export const nodeCaches = { + _cacheStores: {}, + + /** Returns a Promise that resolves to the requested Cache object. */ + open: jest.fn(cacheName => { + // Initialize the store for the cache name if it doesn't exist + if (!nodeCaches._cacheStores[cacheName]) { + nodeCaches._cacheStores[cacheName] = {} + } + + return Promise.resolve({ + add: jest.fn(request => { + const response = new Response(`Response for ${request.url}`) + return nodeCaches.put(request, response, cacheName) + }), + addAll: jest.fn(requests => { + const responses = requests.map(req => new Response(`Response for ${req.url}`)) + return Promise.all(responses.map((response, index) => { + return nodeCaches.put(requests[index], response, cacheName) + })) + }), + delete: jest.fn(request => { + const key = request.url || request + if (nodeCaches._cacheStores[cacheName][key]) { + delete nodeCaches._cacheStores[cacheName][key] + return Promise.resolve(true) + } + return Promise.resolve(false) + }), + match: jest.fn(request => { + const key = request.url || request + return Promise.resolve(nodeCaches._cacheStores[cacheName][key] || undefined) + }), + put: jest.fn((request, response) => { + const key = request.url || request + nodeCaches._cacheStores[cacheName][key] = response + return Promise.resolve() + }), + keys: jest.fn(() => { + return Promise.resolve(Object.keys(nodeCaches._cacheStores[cacheName]).map(key => new Request(key))) + }) + }) + }), + + match: jest.fn((request, cacheName) => { + const key = request.url || request + return Promise.resolve(nodeCaches._cacheStores[cacheName]?.[key] || null) + }), + + delete: jest.fn((request, cacheName) => { + const key = request.url || request + if (nodeCaches._cacheStores[cacheName] && nodeCaches._cacheStores[cacheName][key]) { + delete nodeCaches._cacheStores[cacheName][key] + return Promise.resolve(true) + } + return Promise.resolve(false) + }), + + keys: jest.fn(() => { + // Collect all cache keys across all cache stores + return Object.keys(nodeCaches._cacheStores) + }) +} + +export const nodeRequest = class { + constructor(url, options = {}) { + this.url = url + this.method = options.method || 'GET' + this.headers = new Headers(options.headers) + } +} + +export const nodeResponse = class { + constructor(body, options = {}) { + this.status = options.status || 200 + this.ok = this.status >= 200 && this.status < 300 + this.headers = new Headers(options.headers) + this.bodyUsed = false + + // Ensure the body is a string or Buffer + const readableBody = typeof body === 'string' ? body : body.toString() + + // Create a ReadableStream + const stream = new Readable({ + read() { + this.push(readableBody) // Push the string or buffer into the stream + this.push(null) // Signal the end of the stream + } + }) + + this.body = stream // Assign the readable stream to body + } + + // Define text() method to read from the stream + async text() { + if (this.bodyUsed) { + return Promise.reject(new TypeError('Already read')) + } + this.bodyUsed = true + return new Promise((resolve, reject) => { + const chunks = [] + this.body.on('data', chunk => { + chunks.push(chunk) + }) + this.body.on('end', () => { + resolve(Buffer.concat(chunks).toString()) + }) + this.body.on('error', (err) => { + reject(err) + }) + }) + } + + // You can also implement json() if needed + async json() { + const text = await this.text() + return JSON.parse(text) + } +} + + +export const nodeHeaders = class { + constructor(headers) { + this.map = new Map() + if (headers) { + for (const [key, value] of Object.entries(headers)) { + this.append(key, value) + } + } + } + + append(key, value) { + const existingValue = this.map.get(key.toLowerCase()) + if (existingValue) { + this.map.set(key.toLowerCase(), existingValue + ', ' + value) + } else { + this.map.set(key.toLowerCase(), value) + } + } + + get(key) { + return this.map.get(key.toLowerCase()) || null + } + + has(key) { + return this.map.has(key.toLowerCase()) + } + + set(key, value) { + this.map.set(key.toLowerCase(), value) + } + + delete(key) { + this.map.delete(key.toLowerCase()) + } + + // Additional methods if needed +} diff --git a/test/js/lib/ontology-validation.test.js b/test/js/lib/ontology-validation.test.js new file mode 100644 index 0000000000..f0f06e1715 --- /dev/null +++ b/test/js/lib/ontology-validation.test.js @@ -0,0 +1,28 @@ +const fetch = require('node-fetch') + +import { + fetchOntologies +} from 'lib/validation/ontology-validation' + +import { + nodeCaches, nodeHeaders, nodeRequest, nodeResponse +} from './node-web-api' + +describe('Client-side file validation for AnnData', () => { + beforeAll(() => { + global.fetch = fetch + + global.caches = nodeCaches; + global.Response = nodeResponse + global.Request = nodeRequest + global.Headers = nodeHeaders + }) + + it('Parses minified ontologies', async () => { + const ontologies = await fetchOntologies() + const expectedOntologyNames = ['mondo', 'pato', 'efo', 'uberon', 'ncbitaxon'] + expect(Object.keys(ontologies)).toEqual(expectedOntologyNames) + const expectedSpeciesNames = ['Homo sapiens', 'human'] + expect(ontologies.ncbitaxon['NCBITaxon_9606']).toEqual(expectedSpeciesNames) + }) +}) diff --git a/test/js/lib/validate-anndata.test.js b/test/js/lib/validate-anndata.test.js index ec9e0a5ad8..b89d27c988 100644 --- a/test/js/lib/validate-anndata.test.js +++ b/test/js/lib/validate-anndata.test.js @@ -2,10 +2,15 @@ import { getHdf5File, parseAnnDataFile, getAnnDataHeaders, checkOntologyIdFormat } from 'lib/validation/validate-anndata' +const BASE_URL = 'https://github.com/broadinstitute/single_cell_portal_core/raw/development/test/test_data' + +// TODO: Uncomment this after PR merge +// const BASE_URL = 'https://github.com/broadinstitute/single_cell_portal_core/raw/development/test/test_data/anndata' + describe('Client-side file validation for AnnData', () => { it('Parses AnnData headers', async () => { - // eslint-disable-next-line max-len - const url = 'https://github.com/broadinstitute/single_cell_portal_core/raw/development/test/test_data/anndata_test.h5ad' + const url = `${BASE_URL}/anndata_test.h5ad` + // const url = `${BASE_URL}/valid.h5ad` // Uncomment after PR merge const expectedHeaders = [ '_index', 'biosample_id', @@ -20,22 +25,22 @@ describe('Client-side file validation for AnnData', () => { 'species', 'species__ontology_label' ] - const remoteProps = {url} + const remoteProps = { url } const hdf5File = await getHdf5File(url, remoteProps) const headers = await getAnnDataHeaders(hdf5File) expect(headers).toEqual(expectedHeaders) }) it('Reports AnnData with valid headers as valid', async () => { - // eslint-disable-next-line max-len - const url = 'https://github.com/broadinstitute/single_cell_portal_core/raw/development/test/test_data/anndata_test.h5ad' + const url = `${BASE_URL}/anndata_test.h5ad` + // const url = `${BASE_URL}/valid.h5ad` // Uncomment after PR merge const parseResults = await parseAnnDataFile(url) expect(parseResults.issues).toHaveLength(0) }) it('Reports AnnData with invalid headers as invalid', async () => { - // eslint-disable-next-line max-len - const url = 'https://github.com/broadinstitute/single_cell_portal_core/raw/development/test/test_data/anndata_test_bad_header_no_species.h5ad' + const url = `${BASE_URL}/anndata_test_bad_header_no_species.h5ad` + // const url = `${BASE_URL}/invalid_header_no_species.h5ad` // Uncomment after PR merge const parseResults = await parseAnnDataFile(url) expect(parseResults.issues).toHaveLength(1) @@ -57,10 +62,24 @@ describe('Client-side file validation for AnnData', () => { expect(issues).toHaveLength(0) }) - // TODO: Uncomment this after row-level AnnData parsing PR is merged - // it('Parses AnnData rows and reports invalid ontology IDs', async () => { - // // eslint-disable-next-line max-len - // const url = 'https://github.com/broadinstitute/single_cell_portal_core/raw/development/test/test_data/anndata_test_invalid_disease.h5ad' + it('Parses AnnData rows and reports invalid ontology IDs', async () => { + const url = `${BASE_URL}/anndata_test_invalid_disease.h5ad` + // const url = `${BASE_URL}/invalid_disease_id.h5ad` // Uncomment after PR merge + const parseResults = await parseAnnDataFile(url) + + expect(parseResults.issues).toHaveLength(1) + + const expectedIssue = [ + 'error', + 'ontology:label-lookup-error', + 'Ontology ID "FOO_0000042" is not among accepted ontologies (MONDO, PATO) for key "disease"' + ] + expect(parseResults.issues[0]).toEqual(expectedIssue) + }) + + // // TODO: Uncomment after PR merge + // it('Parses AnnData rows and reports invalid ontology labels', async () => { + // const url = `${BASE_URL}/invalid_disease_label.h5ad` // const parseResults = await parseAnnDataFile(url) // expect(parseResults.issues).toHaveLength(1) @@ -68,7 +87,10 @@ describe('Client-side file validation for AnnData', () => { // const expectedIssue = [ // 'error', // 'ontology:label-lookup-error', - // 'Ontology ID "FOO_0000042" is not among accepted ontologies (MONDO, PATO) for key "disease"' + // 'Invalid disease label "foo". Valid labels for MONDO_0018076: tuberculosis, tuberculosis disease, active tuberculosis, Kochs disease, TB', + // { + // 'subtype': 'ontology:invalid-label' + // } // ] // expect(parseResults.issues[0]).toEqual(expectedIssue) // }) diff --git a/test/js/upload-wizard/file-upload-control.test.js b/test/js/upload-wizard/file-upload-control.test.js index 2db1de4481..7796889169 100644 --- a/test/js/upload-wizard/file-upload-control.test.js +++ b/test/js/upload-wizard/file-upload-control.test.js @@ -1,5 +1,5 @@ import React from 'react' -import { render, screen, cleanup, fireEvent, waitForElementToBeRemoved } from '@testing-library/react' +import { render, screen, cleanup, fireEvent, waitForElementToBeRemoved, waitFor } from '@testing-library/react' import '@testing-library/jest-dom/extend-expect' import { StudyContext } from 'components/upload/upload-utils' @@ -354,6 +354,77 @@ describe('file upload control validates the selected file', () => { expect(screen.queryAllByText('Use bucket path')).toHaveLength(1) const bucketToggle = screen.getByText('Use bucket path') fireEvent.click(bucketToggle) - expect(container.querySelector('#remote_location-input-123')).toBeVisible(); + expect(container.querySelector('#remote_location-input-123')).toBeVisible() + }) + + it('validates file extension on bucket path entries', async () => { + const file = { + _id: '123', + name: '', + status: 'new', + file_type: 'Cluster' + } + render(( + + + + )) + expect(screen.queryAllByText('Use bucket path')).toHaveLength(1) + const bucketToggle = screen.getByText('Use bucket path') + fireEvent.click(bucketToggle) + const bucketPathInput = screen.getByTestId('remote-location-input') + expect(bucketPathInput).toBeVisible() + fireEvent.change(bucketPathInput, { target: { value: 'bad.pdf' } }) + expect(bucketPathInput).toHaveValue('bad.pdf') + await waitFor(() => { + expect(screen.getByTestId('validation-error')).toBeInTheDocument() + expect(screen.getByText('Allowed extensions are .tsv')).toBeVisible() + }) + }) + + it ('clears out previous uploaded file after replace fails to validate', async () => { + const file = { + _id: '123', + name: 'cluster.txt', + status: 'new', + file_type: 'Cluster', + upload_file_name: 'cluster.txt', + uploadSelection: {} // needed to mock a file already being selected + } + const updateFileHolder = { updateFile: () => {} } + const updateFileSpy = jest.spyOn(updateFileHolder, 'updateFile') + render(( + + + + )) + + const replaceLink = screen.getByText('Replace') + expect(replaceLink).toBeVisible() + // test replacing with bad file & clearing out state + // this won't update the button state back to 'Choose file' since updateFile() is mocked above + // unsetting uploadSelection and upload_file_name achieves this in actual usage + fireFileSelectionEvent(screen.getByTestId('file-input'), { + fileName: 'cluster_bad.txt', + content: 'foo,X,Y\nTYPE,numeric,numeric\nCell1,1,0\n' + },true) + await waitForElementToBeRemoved(() => screen.getByTestId('file-validation-spinner')) + expect(updateFileSpy).toHaveBeenLastCalledWith('123', { + uploadSelection: null, + name: '', + upload_file_name: '' + }) + expect(screen.getByTestId('validation-error')).toHaveTextContent('after correcting cluster_bad.txt') + expect(screen.getByTestId('validation-error')) + .toHaveTextContent('First row, first column must be "NAME" (case insensitive). Your value was "foo') }) }) diff --git a/test/models/ann_data_file_info_test.rb b/test/models/ann_data_file_info_test.rb index 8463fc9372..94db34b3dd 100644 --- a/test/models/ann_data_file_info_test.rb +++ b/test/models/ann_data_file_info_test.rb @@ -120,6 +120,17 @@ def generate_id assert_equal %w[X_umap X_tsne], ann_data_info.obsm_key_names end + test 'should set default cluster fragments' do + ann_data_info = AnnDataFileInfo.new + assert ann_data_info.valid? + default_keys = AnnDataIngestParameters::PARAM_DEFAULTS[:obsm_keys] + default_keys.each do |obsm_key_name| + name = obsm_key_name.delete_prefix('X_') + matcher = { data_type: :cluster, name:, obsm_key_name: }.with_indifferent_access + assert ann_data_info.find_fragment(**matcher).present? + end + end + test 'should validate data fragments' do ann_data_info = AnnDataFileInfo.new( data_fragments: [ @@ -224,7 +235,7 @@ def generate_id ] ) assert anndata_info.valid? # invokes validations - exp_frag = anndata_info.data_fragments.first + exp_frag = anndata_info.fragments_by_type(:expression).first assert_nil exp_frag.with_indifferent_access.dig(:expression_file_info, :units) end end diff --git a/test/models/ann_data_ingest_parameters_test.rb b/test/models/ann_data_ingest_parameters_test.rb index 1a3023bc42..734d83cc7d 100644 --- a/test/models/ann_data_ingest_parameters_test.rb +++ b/test/models/ann_data_ingest_parameters_test.rb @@ -56,7 +56,7 @@ class AnnDataIngestParametersTest < ActiveSupport::TestCase assert extraction.send(attr).blank? end - cmd = '--ingest-anndata --anndata-file gs://bucket_id/test.h5ad --obsm-keys ["X_umap", "X_tsne"] --extract ' \ + cmd = '--ingest-anndata --anndata-file gs://bucket_id/test.h5ad --obsm-keys ["X_umap"] --extract ' \ '["cluster", "metadata", "processed_expression", "raw_counts"]' assert_equal cmd, extraction.to_options_array.join(' ') assert_equal 'n2d-highmem-32', extraction.machine_type diff --git a/test/models/home_page_link_test.rb b/test/models/home_page_link_test.rb new file mode 100644 index 0000000000..e405567793 --- /dev/null +++ b/test/models/home_page_link_test.rb @@ -0,0 +1,48 @@ +require "test_helper" + +class HomePageLinkTest < ActiveSupport::TestCase + + before(:all) do + @home_page_link = HomePageLink.create(name: 'Broad Institute', href: 'https://broadinstitute.org') + end + + teardown do + HomePageLink.unpublish! + end + + after(:all) do + HomePageLink.delete_all + end + + test 'should publish/unpublish link' do + assert_nil HomePageLink.published + HomePageLink.publish_last! + assert HomePageLink.published.present? + link = HomePageLink.published + link.unpublish! + assert_nil HomePageLink.published + link.publish! + assert HomePageLink.published.present? + HomePageLink.unpublish! + assert_nil HomePageLink.published + end + + test 'should reset css/color attributes' do + @home_page_link.update(css_class: 'foo', bg_color: '#ff0000') + @home_page_link.reset_css! + @home_page_link.reload + assert_equal HomePageLink::DEFAULT_CSS_CLASS, @home_page_link.css_class + @home_page_link.reset_bg_color! + @home_page_link.reload + assert_equal HomePageLink::DEFAULT_BG_COLOR, @home_page_link.bg_color + end + + test 'should ensure only one link is published' do + assert_not HomePageLink.published.present? + HomePageLink.publish_last! + assert HomePageLink.published.present? + new_link = HomePageLink.new(name: 'Google', href: 'https://google.com', published: true) + assert_not new_link.valid? + assert_includes new_link.errors.attribute_names, :published + end +end diff --git a/test/models/ingest_job_test.rb b/test/models/ingest_job_test.rb index f8e7c13688..4a8d0b4fac 100644 --- a/test/models/ingest_job_test.rb +++ b/test/models/ingest_job_test.rb @@ -9,15 +9,14 @@ class IngestJobTest < ActiveSupport::TestCase user: @user, test_array: @@studies_to_clean) - @basic_study_exp_file = FactoryBot.create(:study_file, + @basic_study_exp_file = FactoryBot.create(:expression_file, name: 'dense.txt', file_type: 'Expression Matrix', - study: @basic_study) + study: @basic_study, + expression_input: { + 'PTEN' => [['A', 0],['B', 3],['C', 1.5]] + }) - @pten_gene = FactoryBot.create(:gene_with_expression, - name: 'PTEN', - study_file: @basic_study_exp_file, - expression_input: [['A', 0],['B', 3],['C', 1.5]]) @basic_study_exp_file.build_expression_file_info(is_raw_counts: false, library_preparation_protocol: 'MARS-seq', modality: 'Transcriptomic: unbiased', @@ -26,11 +25,6 @@ class IngestJobTest < ActiveSupport::TestCase @basic_study_exp_file.upload_file_size = 1024 @basic_study_exp_file.save! - # insert "all cells" array for this expression file - DataArray.create!(study_id: @basic_study.id, study_file_id: @basic_study_exp_file.id, values: %w(A B C), - name: "#{@basic_study_exp_file.name} Cells", array_type: 'cells', linear_data_type: 'Study', - linear_data_id: @basic_study.id, array_index: 0, cluster_name: @basic_study_exp_file.name) - @other_matrix = FactoryBot.create(:study_file, name: 'dense_2.txt', file_type: 'Expression Matrix', @@ -39,6 +33,13 @@ class IngestJobTest < ActiveSupport::TestCase modality: 'Transcriptomic: unbiased', biosample_input_type: 'Whole cell') @other_matrix.upload_file_size = 2048 @other_matrix.save! + @basic_study.reload + end + + teardown do + @basic_study.default_options[:annotation] = nil + @basic_study.save + @basic_study_exp_file.update(parse_status: 'parsed') end test 'should hold ingest until other matrix validates' do @@ -89,7 +90,25 @@ class IngestJobTest < ActiveSupport::TestCase test 'should gather job properties to report to mixpanel' do # positive test - job = IngestJob.new(study: @basic_study, study_file: @basic_study_exp_file, user: @user, action: :ingest_expression) + study = FactoryBot.create(:detached_study, + name_prefix: 'IngestJob Mixpanel Test', + user: @user, + test_array: @@studies_to_clean) + + study_file = FactoryBot.create(:expression_file, + name: 'matrix.txt', + study:, + expression_input: { + 'Phex' => [['A', 1],['B', 2],['C', 0.5]] + }) + + study_file.build_expression_file_info(is_raw_counts: false, + library_preparation_protocol: "10x 5' v3", + modality: 'Transcriptomic: unbiased', + biosample_input_type: 'Whole cell') + study_file.upload_file_size = 1.megabyte + study_file.save! + job = IngestJob.new(study:, study_file:, user: @user, action: :ingest_expression) mock = Minitest::Mock.new now = DateTime.now.in_time_zone mock_metadata = { @@ -112,20 +131,20 @@ class IngestJobTest < ActiveSupport::TestCase mock.expect :error, nil mock.expect :done?, true - cells = @basic_study.expression_matrix_cells(@basic_study_exp_file) - num_cells = cells.present? ? cells.count : 0 + cells = study.expression_matrix_cells(study_file) + num_cells = cells.count ApplicationController.life_sciences_api_client.stub :get_pipeline, mock do expected_outputs = { perfTime: 60000, - fileType: @basic_study_exp_file.file_type, - fileSize: @basic_study_exp_file.upload_file_size, - fileName: @basic_study_exp_file.name, + fileType: study_file.file_type, + fileSize: study_file.upload_file_size, + fileName: study_file.name, trigger: 'upload', action: :ingest_expression, - studyAccession: @basic_study.accession, + studyAccession: study.accession, jobStatus: 'success', - numGenes: @basic_study.genes.count, + numGenes: study.genes.count, is_raw_counts: false, numCells: num_cells, machineType: 'n2d-highmem-4', @@ -139,7 +158,15 @@ class IngestJobTest < ActiveSupport::TestCase end # negative test - job = IngestJob.new(study: @basic_study, study_file: @other_matrix, user: @user, action: :ingest_expression) + other_file = FactoryBot.create(:study_file, + name: 'matrix_2.txt', + file_type: 'Expression Matrix', + study:) + other_file.build_expression_file_info(is_raw_counts: false, library_preparation_protocol: 'MARS-seq', + modality: 'Transcriptomic: unbiased', biosample_input_type: 'Whole cell') + other_file.upload_file_size = 2048 + other_file.save! + job = IngestJob.new(study:, study_file: other_file, user: @user, action: :ingest_expression) mock = Minitest::Mock.new now = DateTime.now.in_time_zone mock_metadata = { @@ -164,12 +191,12 @@ class IngestJobTest < ActiveSupport::TestCase ApplicationController.life_sciences_api_client.stub :get_pipeline, mock do expected_outputs = { perfTime: 120000, - fileType: @other_matrix.file_type, - fileSize: @other_matrix.upload_file_size, - fileName: @other_matrix.name, + fileType: other_file.file_type, + fileSize: other_file.upload_file_size, + fileName: other_file.name, trigger: "upload", action: :ingest_expression, - studyAccession: @basic_study.accession, + studyAccession: study.accession, jobStatus: 'failed', numCells: 0, is_raw_counts: false, @@ -187,17 +214,27 @@ class IngestJobTest < ActiveSupport::TestCase test 'should identify AnnData parses with extraction in mixpanel' do # parsed AnnData - ann_data_file = FactoryBot.create(:ann_data_file, name: 'test.h5ad', study: @basic_study) - ann_data_file.ann_data_file_info.reference_file = false - ann_data_file.ann_data_file_info.data_fragments = [ - { _id: BSON::ObjectId.new.to_s, data_type: :cluster, obsm_key_name: 'X_umap', name: 'UMAP' } - ] - ann_data_file.upload_file_size = 1.megabyte - ann_data_file.save + ann_data_file = FactoryBot.create(:ann_data_file, + name: 'test.h5ad', + study: @basic_study, + upload_file_size: 1.megabyte, + cell_input: %w[A B C D], + has_raw_counts: true, + reference_file: false, + annotation_input: [ + { name: 'disease', type: 'group', values: %w[cancer cancer normal normal] } + ], + coordinate_input: [ + { X_umap: { x: [1, 2, 3, 4], y: [5, 6, 7, 8] } } + ], + expression_input: { + 'phex' => [['A', 0.3], ['B', 1.0], ['C', 0.5], ['D', 0.1]] + }) params_object = AnnDataIngestParameters.new( anndata_file: ann_data_file.gs_url, obsm_keys: ann_data_file.ann_data_file_info.obsm_key_names, - file_size: ann_data_file.upload_file_size + file_size: ann_data_file.upload_file_size, extract_raw_counts: true ) + assert params_object.extract.include?('raw_counts') job = IngestJob.new( study: @basic_study, study_file: ann_data_file, user: @user, action: :ingest_anndata, params_object: ) @@ -232,7 +269,7 @@ class IngestJobTest < ActiveSupport::TestCase studyAccession: @basic_study.accession, jobStatus: 'success', referenceAnnDataFile: false, - extractedFileTypes: %w[cluster metadata processed_expression], + extractedFileTypes: %w[cluster metadata processed_expression raw_counts], machineType: 'n2d-highmem-4', bootDiskSizeGb: 300, exitStatus: 0 @@ -326,9 +363,9 @@ class IngestJobTest < ActiveSupport::TestCase } ] }, - createTime: (now - 1.minutes).to_default_s, - startTime: (now - 1.minutes).to_default_s, - endTime: now.to_default_s + createTime: (now - 1.minutes).to_s, + startTime: (now - 1.minutes).to_s, + endTime: now.to_s }.with_indifferent_access pipeline_mock = MiniTest::Mock.new @@ -484,40 +521,44 @@ class IngestJobTest < ActiveSupport::TestCase end test 'should set default annotation even if not visualizable' do - assert @basic_study.default_annotation.nil? + study = FactoryBot.create(:detached_study, + name_prefix: 'Default Annotation Test', + user: @user, + test_array: @@studies_to_clean) + assert study.default_annotation.nil? # test metadata file with a single annotation with only one unique value metadata_file = FactoryBot.create(:metadata_file, name: 'metadata.txt', - study: @basic_study, + study:, cell_input: %w[A B C], annotation_input: [ { name: 'species', type: 'group', values: %w[dog dog dog] } ]) - job = IngestJob.new(study: @basic_study, study_file: metadata_file, user: @user, action: :ingest_cell_metadata) + job = IngestJob.new(study:, study_file: metadata_file, user: @user, action: :ingest_cell_metadata) job.set_study_default_options - @basic_study.reload - assert_equal 'species--group--invalid', @basic_study.default_annotation + study.reload + assert_equal 'species--group--invalid', study.default_annotation # reset default annotation, then test cluster file with a single annotation with only one unique value - @basic_study.cell_metadata.destroy_all - @basic_study.default_options = {} - @basic_study.save - assert @basic_study.default_annotation.nil? - assert @basic_study.default_cluster.nil? + study.cell_metadata.destroy_all + study.default_options = {} + study.save + assert study.default_annotation.nil? + assert study.default_cluster.nil? cluster_file = FactoryBot.create(:cluster_file, - name: 'cluster.txt', study: @basic_study, + name: 'cluster.txt', study:, cell_input: { x: [1, 4, 6], y: [7, 5, 3], cells: %w[A B C] }, annotation_input: [{ name: 'foo', type: 'group', values: %w[bar bar bar] }]) - job = IngestJob.new(study: @basic_study, study_file: cluster_file, user: @user, action: :ingest_cluster) + job = IngestJob.new(study:, study_file: cluster_file, user: @user, action: :ingest_cluster) job.set_study_default_options - @basic_study.reload - cluster = @basic_study.cluster_groups.first - assert_equal cluster, @basic_study.default_cluster - assert_equal 'foo--group--invalid', @basic_study.default_annotation + study.reload + cluster = study.cluster_groups.first + assert_equal cluster, study.default_cluster + assert_equal 'foo--group--invalid', study.default_annotation end test 'should launch DE jobs if study is eligible' do @@ -673,7 +714,7 @@ class IngestJobTest < ActiveSupport::TestCase ls_mock.expect :get_pipeline, pipeline_mock, [{ name: pipeline_name }] ApplicationController.stub :firecloud_client, mock do ApplicationController.stub :life_sciences_api_client, ls_mock do - job.handle_ingest_failure + job.handle_ingest_failure('parse failure') cluster_file.reload study.reload cluster.reload @@ -714,7 +755,7 @@ class IngestJobTest < ActiveSupport::TestCase ls_mock.expect :get_pipeline, pipeline_mock, [{ name: pipeline_name }] ApplicationController.stub :firecloud_client, mock do ApplicationController.stub :life_sciences_api_client, ls_mock do - failed_job.handle_ingest_failure + failed_job.handle_ingest_failure('parse failure') mock.verify end end @@ -760,6 +801,77 @@ class IngestJobTest < ActiveSupport::TestCase end end + test 'should ensure email delivery on special action failures' do + study = FactoryBot.create(:detached_study, + name_prefix: 'Special Action Email', + user: @user, + test_array: @@studies_to_clean) + study_file = FactoryBot.create(:ann_data_file, + name: 'matrix.h5ad', + study:, + upload_file_size: 1.megabyte, + cell_input: %w[A B C D], + annotation_input: [ + { name: 'disease', type: 'group', values: %w[cancer cancer normal normal] } + ], + coordinate_input: [ + { X_umap: { x: [1, 2, 3, 4], y: [5, 6, 7, 8] } } + ]) + annotation_file = 'gs://test_bucket/anndata/h5ad_frag.metadata.tsv' + cluster_file = 'gs://test_bucket/anndata/h5ad_frag.cluster.X_umap.tsv' + params_object = DifferentialExpressionParameters.new( + matrix_file_path: 'gs://test_bucket/matrix.h5ad', matrix_file_type: 'h5ad', file_size: study_file.upload_file_size, + annotation_file:, cluster_file:, cluster_name: 'umap', annotation_name: 'disease', annotation_scope: 'study' + ) + pipeline_name = SecureRandom.uuid + job = IngestJob.new( + pipeline_name:, study:, study_file:, user: @user, action: :differential_expression, params_object: + ) + now = DateTime.now.in_time_zone + mock_metadata = { + events: [ + { timestamp: now.to_s }, + { timestamp: (now + 2.minutes).to_s, containerStopped: { exitStatus: 1 } } + ], + pipeline: { + actions: [ + { + commands: [ + 'python', 'ingest_pipeline.py', '--study-id', study.id.to_s, '--study-file-id', + study_file.id.to_s, 'differential_expression', '--differential-expression', '--cluster-file', cluster_file, + '--annotation-file', annotation_file, '--cluster-name', 'umap', '--annotation-name', 'disease', + '--annotation-scope', 'study' + ] + } + ], + resources: { + virtualMachine: { + machineType: 'n2d-highmem-8', + bootDiskSizeGb: 300 + } + } + }, + createTime: (now - 3.minutes).to_s, + startTime: now.to_s, + endTime: now.to_s + }.with_indifferent_access + mock = Minitest::Mock.new + 6.times { mock.expect :metadata, mock_metadata } + 3.times do + mock.expect :error, { code: 1, message: 'mock message' } + mock.expect :done?, true + end + email_mock = Minitest::Mock.new + email_mock.expect :deliver_now, true + ApplicationController.life_sciences_api_client.stub :get_pipeline, mock do + SingleCellMailer.stub :notify_admin_parse_fail, email_mock do + job.poll_for_completion + mock.verify + email_mock.verify + end + end + end + test 'should always unset subsampling flags' do study = FactoryBot.create(:detached_study, name_prefix: 'Subsample Flag Test', diff --git a/test/services/download_quota_service_test.rb b/test/services/download_quota_service_test.rb index e65f7ec387..020af3acdc 100644 --- a/test/services/download_quota_service_test.rb +++ b/test/services/download_quota_service_test.rb @@ -57,4 +57,11 @@ class DownloadQuotaServiceTest < ActiveSupport::TestCase ) assert_equal 1.megabyte, DownloadQuotaService.download_quota end + + test 'should handle nil file size' do + starting_quota = @user.daily_download_quota + DownloadQuotaService.increment_user_quota(@user, nil) + @user.reload + assert_equal starting_quota, @user.daily_download_quota + end end diff --git a/test/test_data/anndata_test_invalid_disease.h5ad b/test/test_data/anndata/invalid_disease_id.h5ad similarity index 100% rename from test/test_data/anndata_test_invalid_disease.h5ad rename to test/test_data/anndata/invalid_disease_id.h5ad diff --git a/test/test_data/anndata/invalid_disease_label.h5ad b/test/test_data/anndata/invalid_disease_label.h5ad new file mode 100644 index 0000000000..23cbfa526c Binary files /dev/null and b/test/test_data/anndata/invalid_disease_label.h5ad differ diff --git a/test/test_data/anndata_test_bad_header_no_species.h5ad b/test/test_data/anndata/invalid_header_no_species.h5ad similarity index 100% rename from test/test_data/anndata_test_bad_header_no_species.h5ad rename to test/test_data/anndata/invalid_header_no_species.h5ad diff --git a/test/test_data/anndata_test.h5ad b/test/test_data/anndata/valid.h5ad similarity index 100% rename from test/test_data/anndata_test.h5ad rename to test/test_data/anndata/valid.h5ad diff --git a/yarn.lock b/yarn.lock index 08a604739d..2ed24b0665 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8305,9 +8305,9 @@ rimraf@^3.0.0: glob "^7.1.3" rollup@^3.27.1: - version "3.29.4" - resolved "https://registry.npmjs.org/rollup/-/rollup-3.29.4.tgz" - integrity sha512-oWzmBZwvYrU0iJHtDmhsm662rC15FRXmcjCk1xD771dFDx5jJ02ufAQQTn0etB2emNk4J9EZg/yWKpsn9BWGRw== + version "3.29.5" + resolved "https://registry.yarnpkg.com/rollup/-/rollup-3.29.5.tgz#8a2e477a758b520fb78daf04bca4c522c1da8a54" + integrity sha512-GVsDdsbJzzy4S/v3dqWPJ7EfvZJfCHiDqe80IyrF59LYuP+e6U1LJoUqeuqRbwAWoMNoXivMNeNAOf5E22VA1w== optionalDependencies: fsevents "~2.3.2" @@ -9387,10 +9387,10 @@ vite-plugin-ruby@^3.0.5: debug "^4.3.4" fast-glob "^3.2.12" -vite@^4.0.0: - version "4.5.3" - resolved "https://registry.npmjs.org/vite/-/vite-4.5.3.tgz" - integrity sha512-kQL23kMeX92v3ph7IauVkXkikdDRsYMGTVl5KY2E9OY4ONLvkHf04MDTbnfo6NKxZiDLWzVpP5oTa8hQD8U3dg== +vite@^4.5.5: + version "4.5.5" + resolved "https://registry.yarnpkg.com/vite/-/vite-4.5.5.tgz#639b9feca5c0a3bfe3c60cb630ef28bf219d742e" + integrity sha512-ifW3Lb2sMdX+WU91s3R0FyQlAyLxOzCSCP37ujw0+r5POeHPwe6udWVIElKQq8gk3t7b8rkmvqC6IHBpCff4GQ== dependencies: esbuild "^0.18.10" postcss "^8.4.27"