Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release 1.3.0 #1274

Merged
merged 34 commits into from
Nov 23, 2021
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
0ac4c27
changing associations to :has_and_belongs_to_many
bistline Nov 10, 2021
561a789
adding migration to reassign for new associations
bistline Nov 10, 2021
679414b
changing params to accept arrays
bistline Nov 10, 2021
6bfd16a
fixing queries for new associations
bistline Nov 10, 2021
6bc927b
initial work on including Azul results in bulk download
bistline Nov 16, 2021
09b5a73
finishing up new collection UX
bistline Nov 16, 2021
6f80761
adding new tests
bistline Nov 16, 2021
0763f1d
finishing tests, refining curator email functionality
bistline Nov 16, 2021
cbf305c
fixing synthetic_branding_group_populator.rb
bistline Nov 16, 2021
fddf533
fixing user assignment
bistline Nov 16, 2021
815bdb6
fixing assignments
bistline Nov 16, 2021
5c7708e
test fixes
bistline Nov 17, 2021
527c124
more test fixes
bistline Nov 17, 2021
3512a9d
finishing up bulk download implementation for Azul
bistline Nov 17, 2021
e6441de
addressing PR comments
bistline Nov 17, 2021
9ad3dad
updating migration comment
bistline Nov 18, 2021
70eb474
addressing more PR comments
bistline Nov 18, 2021
cfb3f20
optimize for memory via lazy-load queries
bistline Nov 18, 2021
da66434
Merge branch 'development' into jb-collection-refactor
bistline Nov 18, 2021
26efc9e
fixing test regression re: merge w/ development
bistline Nov 18, 2021
6fa864a
adding in metrics for reporting (download source, file counts for Azul)
bistline Nov 18, 2021
aa43fb1
fixing disease corner case in search facet matching
bistline Nov 18, 2021
c007fb7
fixing broken link
bistline Nov 18, 2021
735d0f0
address PR comments re: Mixpanel properties
bistline Nov 18, 2021
79b33a5
working on test regressions
bistline Nov 19, 2021
220b04b
fixing test regression re: new generate_curl_config process
bistline Nov 19, 2021
4d0862f
fixing test idempotency, ensuring download context is set appropriately
bistline Nov 19, 2021
0f7d781
consolidating scatter plot tests, adding cache test
devonbush Nov 19, 2021
55d75cf
Merge pull request #1265 from broadinstitute/jb-collection-refactor
bistline Nov 22, 2021
26fcb3b
Merge pull request #1269 from broadinstitute/jb-azul-bulk-download
bistline Nov 22, 2021
4e840f3
Update react-scripts to fix alert re immer
eweitz Nov 22, 2021
5558d97
Merge pull request #1273 from broadinstitute/ew-secure-immer
eweitz Nov 23, 2021
0c1ed14
clarifying test comments
devonbush Nov 23, 2021
5aa1b90
Merge pull request #1270 from broadinstitute/db-bugfix-tests
devonbush Nov 23, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
# SINGLE CELL PORTAL README

[![codecov](https://codecov.io/gh/broadinstitute/single_cell_portal_core/branch/development/graph/badge.svg?token=HMWE5BO2a4)](https://codecov.io/gh/broadinstitute/single_cell_portal_core)
[![Build Status](https://travis-ci.com/broadinstitute/single_cell_portal_core.svg?branch=development)](https://travis-ci.com/broadinstitute/single_cell_portal_core)

[![Build Status](https://app.travis-ci.com/broadinstitute/single_cell_portal_core.svg?branch=development)](https://app.travis-ci.com/broadinstitute/single_cell_portal_core)
## SETUP

This application is built and deployed using [Docker](https://www.docker.com), specifically native
Expand Down
21 changes: 16 additions & 5 deletions app/controllers/api/v1/bulk_download_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,14 @@ def auth_code
half_hour = 1800 # seconds

totat = current_api_user.create_totat(half_hour, api_v1_bulk_download_generate_curl_config_path)
valid_params = params.permit({ file_ids: [], tdr_files: {} }).to_h
valid_params = params.permit({ file_ids: [], azul_files: {} }).to_h

# for now, we don't do any permissions validation on the param values -- we'll do that during the actual download, since
# quota/files/permissions may change between the creation of the download and the actual download.
auth_download = DownloadRequest.create!(
auth_code: totat[:totat],
file_ids: valid_params[:file_ids],
tdr_files: valid_params[:tdr_files],
azul_files: valid_params[:azul_files],
user_id: current_api_user.id
)
auth_code_response = {
Expand Down Expand Up @@ -245,6 +245,13 @@ def drs_info
key :description, 'Name of directory folder to download (for single-study bulk download only), can be "all"'
key :required, false
end
parameter do
key :name, :context
key :type, :string
key :in, :query
key :description, 'Context of the download, its scope: either "study" or "global"'
key :required, false
end
response 200 do
key :description, 'Curl configuration file with signed URLs for requested data'
key :type, :string
Expand Down Expand Up @@ -274,15 +281,18 @@ def drs_info
def generate_curl_config
valid_accessions = []
file_ids = []
tdr_files = {}
azul_files = {}

# determine if this a single-study bulk download (from download tab) or from home page search
search_context = params[:context] || 'unknown'

# branch based on whether they provided a download_id, file_ids, or accessions
if params[:download_id]
download_req = DownloadRequest.find(params[:download_id])
render json: { error: 'Invalid download_id provided' }, status: 400 and return if download_req.blank?

file_ids = download_req.file_ids
tdr_files = download_req.tdr_files
azul_files = download_req.azul_files
elsif params[:file_ids]
begin
file_ids = RequestUtils.validate_id_list(params[:file_ids])
Expand Down Expand Up @@ -344,7 +354,8 @@ def generate_curl_config
user: current_api_user,
study_bucket_map: bucket_map,
output_pathname_map: pathname_map,
tdr_files: tdr_files)
azul_files: azul_files,
context: search_context)
end_time = Time.zone.now
runtime = TimeDifference.between(start_time, end_time).humanize
logger.info "Curl configs generated for studies #{valid_accessions}, #{files_requested.size + directory_files.size} total files"
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v1/search_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ def index

# filter results by branding group, if specified
if @selected_branding_group.present?
@viewable = @viewable.where(branding_group_id: @selected_branding_group.id)
@viewable = @viewable.where(:branding_group_ids.in => [@selected_branding_group.id])
end
# variable for determining how we will sort search results for relevance
sort_type = :none
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v1/studies_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ def check_study_view_permission
# study params list
def study_params
params.require(:study).permit(:name, :description, :public, :embargo, :use_existing_workspace, :firecloud_workspace,
:firecloud_project, :branding_group_id, :cell_count, :gene_count, :view_order,
:firecloud_project, :cell_count, :gene_count, :view_order, branding_group_ids: [],
study_shares_attributes: [:id, :_destroy, :email, :permission],
study_detail_attributes: [:id, :full_description],
:default_options => [:cluster, :annotation, :color_profile, :expression_label, :deliver_emails,
Expand Down
42 changes: 34 additions & 8 deletions app/controllers/branding_groups_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ class BrandingGroupsController < ApplicationController
before_action :set_branding_group, only: [:show, :edit, :update, :destroy]
before_action except: [:list_navigate] do
authenticate_user!
authenticate_admin
end

before_action :authenticate_curator, only: [:show, :edit, :update]
before_action :authenticate_admin, only: [:index, :create, :destroy]

# GET /branding_groups
# GET /branding_groups.json
def index
Expand All @@ -13,11 +15,7 @@ def index

# show a list for display and linking, editable only if the user has appropriate permissions
def list_navigate
@editable_branding_groups = []
@branding_groups = BrandingGroup.visible_groups_to_user(current_user)
if current_user.present?
@editable_branding_groups = current_user.available_branding_groups.to_a
end
end

# GET /branding_groups/1
Expand All @@ -37,7 +35,10 @@ def edit
# POST /branding_groups
# POST /branding_groups.json
def create
@branding_group = BrandingGroup.new(branding_group_params)
clean_params = branding_group_params.to_h
clean_params[:users] = self.class.merge_curator_params(params[:curator_emails], nil, current_user)
clean_params.delete(:user_ids)
@branding_group = BrandingGroup.new(clean_params)

respond_to do |format|
if @branding_group.save
Expand Down Expand Up @@ -65,6 +66,12 @@ def update
end
# delete the param since it is not a real model param
clean_params.delete("reset_#{image_name}")

# merge in curator params
clean_params[:users] = self.class.merge_curator_params(
params[:curator_emails], @branding_group, current_user
)
clean_params.delete(:user_ids)
end

if @branding_group.update(clean_params)
Expand All @@ -90,6 +97,18 @@ def destroy
end
end

# helper to merge in the list of curators into the :users parameter
# will prevent curator from removing themselves from the collection
def self.merge_curator_params(curator_list, collection, user)
curators = curator_list.split(',').map(&:strip)
users = curators.map { |email| User.find_by(email: email) }.compact
return users if collection.nil?

# ensure current user cannot accidentally remove themselves from the list if this is an update
users << user if collection.users.include?(user) && !users.include?(user)
users
end

private

# Use callbacks to share common setup or constraints between actions.
Expand All @@ -99,8 +118,15 @@ def set_branding_group

# Never trust parameters from the scary internet, only allow the permit list through.
def branding_group_params
params.require(:branding_group).permit(:name, :tag_line, :public, :background_color, :font_family, :font_color, :user_id,
params.require(:branding_group).permit(:name, :tag_line, :public, :background_color, :font_family, :font_color,
:splash_image, :banner_image, :footer_image, :external_link_url, :external_link_description,
:reset_splash_image, :reset_footer_image, :reset_banner_image)
:reset_splash_image, :reset_footer_image, :reset_banner_image,
user_ids: [])
end

def authenticate_curator
unless @branding_group.can_edit?(current_user)
redirect_to collection_list_navigate_path, alert: 'You do not have permission to perform that action' and return
end
end
end
3 changes: 2 additions & 1 deletion app/controllers/studies_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1089,7 +1089,8 @@ def set_study
# study params list
def study_params
params.require(:study).permit(:name, :description, :public, :user_id, :embargo, :use_existing_workspace, :firecloud_workspace,
:firecloud_project, :branding_group_id, study_shares_attributes: [:id, :_destroy, :email, :permission],
:firecloud_project, branding_group_ids: [],
study_shares_attributes: [:id, :_destroy, :email, :permission],
external_resources_attributes: [:id, :_destroy, :title, :description, :url, :publication_url],
study_detail_attributes: [:id, :full_description])
end
Expand Down
6 changes: 6 additions & 0 deletions app/helpers/branding_groups_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,10 @@ def web_safe_fonts
]
]
end

def get_published_label(branding_group)
text = branding_group.public ? 'published' : 'private'
class_name = branding_group.public ? 'success' : 'danger'
"<span class='label label-#{class_name}'>#{text}</span>".html_safe
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ export default function DownloadButton({ searchResults={} }) {
/** Note that we are reading the TDR file information from the search results object, which
* means we are reliant on the TDR results being on the current page. Once we begin paging/sorting
* TDR results, this approach will have to be revisited */
const tdrFileInfo = searchResults.studies
?.filter(result => result.study_source === 'TDR')
const azulFileInfo = searchResults.studies
?.filter(result => result.study_source === 'HCA')
?.map(result => ({
accession: result.accession,
name: result.name,
study_source: 'TDR',
study_source: 'HCA',
description: result.description,
hca_project_id: result.hca_project_id,
studyFiles: result.file_information
Expand All @@ -72,7 +72,7 @@ export default function DownloadButton({ searchResults={} }) {
<DownloadSelectionModal
show={showModal}
setShow={setShowModal}
tdrFileInfo={tdrFileInfo}
azulFileInfo={azulFileInfo}
studyAccessions={matchingAccessions}/> }
</>
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { fetchAuthCode, stringifyQuery } from 'lib/scp-api'

/** component for rendering a copyable bulk download command for an array of file ids.
Queries the server to retrieve the appropriate auth code. */
export default function DownloadCommand({ fileIds=[], tdrFiles }) {
export default function DownloadCommand({ fileIds=[], azulFiles }) {
const [isLoading, setIsLoading] = useState(true)
const [authInfo, setAuthInfo] = useState({ authCode: null, timeInterval: 3000 })
const [refreshNum, setRefreshNum] = useState(0)
Expand All @@ -20,7 +20,7 @@ export default function DownloadCommand({ fileIds=[], tdrFiles }) {

useEffect(() => {
setIsLoading(true)
fetchAuthCode(fileIds, tdrFiles).then(result => {
fetchAuthCode(fileIds, azulFiles).then(result => {
setAuthInfo(result)
setIsLoading(false)
})
Expand Down Expand Up @@ -84,7 +84,8 @@ export default function DownloadCommand({ fileIds=[], tdrFiles }) {
function getDownloadCommand(authCode, downloadId) {
const queryParams = {
auth_code: authCode,
download_id: downloadId
download_id: downloadId,
context: 'global' // this is a search-based bulk download request
}
const queryString = stringifyQuery(queryParams)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,57 +10,51 @@ import { bytesToSize } from 'lib/stats'

import { fetchDownloadInfo, fetchDrsInfo } from 'lib/scp-api'

const TDR_COLUMNS = ['project_manifest', 'analysis', 'sequence']
const AZUL_COLUMNS = ['project_manifest', 'analysis', 'sequence']
const SCP_COLUMNS = ['matrix', 'metadata', 'cluster']

/**
* a modal that, given a list of study accessions, allows a user to select/deselect
* studies and file types for download. This queries the bulk_download/summary API method
* to retrieve the list of study details and available files
*/
export default function DownloadSelectionModal({ studyAccessions, tdrFileInfo, show, setShow }) {
export default function DownloadSelectionModal({ studyAccessions, azulFileInfo, show, setShow }) {
const [isLoading, setIsLoading] = useState(true)
const [isLoadingTDR, setIsLoadingTDR] = useState(true)
const [isLoadingAzul, setIsLoadingAzul] = useState(true)
const [downloadInfo, setDownloadInfo] = useState([])
const [selectedBoxes, setSelectedBoxes] = useState()
const [downloadInfoTDR, setDownloadInfoTDR] = useState([])
const [selectedBoxesTDR, setSelectedBoxesTDR] = useState()
const [downloadInfoAzul, setDownloadInfoAzul] = useState([])
const [selectedBoxesAzul, setSelectedBoxesAzul] = useState()
const [stepNum, setStepNum] = useState(1)

const scpAccessions = studyAccessions.filter(accession => accession.startsWith('SCP'))
const tdrAccessions = studyAccessions.filter(accession => !accession.startsWith('SCP'))
const showTDRSelectionPane = tdrAccessions.length > 0
const azulAccessions = studyAccessions.filter(accession => !accession.startsWith('SCP'))
const showAzulSelectionPane = azulAccessions.length > 0
const { fileCount, fileSize } = getSelectedFileStats(downloadInfo, selectedBoxes, isLoading)
const { fileCount: fileCountTDR, fileSize: fileSizeTDR } =
getSelectedFileStats(downloadInfoTDR, selectedBoxesTDR, isLoadingTDR)
const prettyBytes = bytesToSize(fileSize + fileSizeTDR)
const { fileCount: fileCountAzul, fileSize: fileSizeAzul } =
getSelectedFileStats(downloadInfoAzul, selectedBoxesAzul, isLoadingAzul)
const prettyBytes = bytesToSize(fileSize + fileSizeAzul)
const selectedFileIds = getSelectedFileHandles(downloadInfo, selectedBoxes)
const selectedTdrFiles = getSelectedFileHandles(downloadInfoTDR, selectedBoxesTDR, true)

/** For TDR studies, we will know from the tdrFileInfo object how many files of each type there are
* But we need to query the drsInfo so that we can get the file sizes and download urls */
function initializeTDRTable() {
setSelectedBoxesTDR(newSelectedBoxesState(tdrFileInfo, TDR_COLUMNS))
setDownloadInfoTDR(tdrFileInfo)
setIsLoadingTDR(false)
}
const selectedAzulFiles = getSelectedFileHandles(downloadInfoAzul, selectedBoxesAzul, true)

/**
render bulk download table for SCP & TDR/HCA studies
render bulk download table for SCP & HCA studies
*/
function renderFileTables(result=[]) {
setSelectedBoxes(newSelectedBoxesState(result, SCP_COLUMNS))
setDownloadInfo(result)
setIsLoading(false)
if (showTDRSelectionPane) {
initializeTDRTable()
if (showAzulSelectionPane) {
setSelectedBoxesAzul(newSelectedBoxesState(azulFileInfo, AZUL_COLUMNS))
setDownloadInfoAzul(azulFileInfo)
setIsLoadingAzul(false)
}
}

useEffect(() => {
if (show) {
setIsLoading(true)
setIsLoadingTDR(true)
setIsLoadingAzul(true)
fetchDownloadInfo(scpAccessions).then(result => {
renderFileTables(result)
})
Expand All @@ -73,7 +67,7 @@ export default function DownloadSelectionModal({ studyAccessions, tdrFileInfo, s
data-analytics-name="download-modal-next">
NEXT
</button>
if (fileCount + fileCountTDR === 0) {
if (fileCount + fileCountAzul === 0) {
downloadButton = <button className="btn btn-primary" disabled="disabled">
No files selected
</button>
Expand Down Expand Up @@ -116,23 +110,23 @@ export default function DownloadSelectionModal({ studyAccessions, tdrFileInfo, s
selectedBoxes={selectedBoxes}
setSelectedBoxes={setSelectedBoxes}
columnTypes={SCP_COLUMNS}/>
{ showTDRSelectionPane &&
{ showAzulSelectionPane &&
<div>
<h4>Human Cell Atlas studies</h4>
<DownloadSelectionTable
isLoading={isLoadingTDR}
downloadInfo={downloadInfoTDR}
selectedBoxes={selectedBoxesTDR}
setSelectedBoxes={setSelectedBoxesTDR}
columnTypes={TDR_COLUMNS}/>
isLoading={isLoadingAzul}
downloadInfo={downloadInfoAzul}
selectedBoxes={selectedBoxesAzul}
setSelectedBoxes={setSelectedBoxesAzul}
columnTypes={AZUL_COLUMNS}/>
</div>
}
</div>
}
{ stepNum === 2 && <DownloadCommand
closeParent={() => setShow(false)}
fileIds={selectedFileIds}
tdrFiles={selectedTdrFiles}/> }
azulFiles={selectedAzulFiles}/> }
{ !isLoading &&
<div className="download-size-message">
<label htmlFor="download-size-amount">Total size</label>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,9 @@ export function getSelectedFileStats(downloadInfo, selectedBoxes, isLoading) {
*/
export function getFileStats(study, fileTypes) {
const files = study.studyFiles.filter(file => fileTypes.includes(file.file_type))
const fileCount = files.length
const fileCount = study.study_source === 'HCA' ? files.reduce((sum, studyFile) => {
return sum + studyFile.count
}, 0) : files.length
const fileSize = files.reduce((sum, studyFile) => {
return sum + (studyFile.upload_file_size ? studyFile.upload_file_size : 0)
}, 0)
Expand All @@ -283,7 +285,16 @@ export function getSelectedFileHandles(downloadInfo, selectedBoxes, hashByStudy=
if (selectedBoxes.studies[index][colType]) {
const filesOfType = study.studyFiles.filter(file => COLUMNS[colType].types.includes(file.file_type))
{/* eslint-disable-next-line max-len */}
const selectedHandles = filesOfType.map(file => hashByStudy ? { drs_id: file.drs_id, url: file.url, name: file.name, file_type: file.file_type } : file.id)
const selectedHandles = filesOfType.map(file => {
if (hashByStudy) {
return {
project_id: file.project_id, name: file.name, file_format: file.file_format,
file_type: file.file_type, count: file.count
}
} else {
return file.id
}
})
if (hashByStudy) {
fileHandles[study.accession].push(...selectedHandles)
} else {
Expand Down
Loading