Release 1.109.0

Gemfile

@@ -111,6 +111,7 @@ group :development, :test do
   gem 'flamegraph'
   gem 'stackprof' # ruby 2.1+ only
   gem 'memory_profiler'
+  gem 'openssl'
 end
 
 group :test do

Gemfile.lock

@@ -365,6 +365,7 @@ GEM
     omniauth-rails_csrf_protection (1.0.0)
       actionpack (>= 4.2)
       omniauth (~> 2.0)
+    openssl (3.3.0)
     orm_adapter (0.5.0)
     os (1.1.4)
     ostruct (0.6.1)
@@ -616,6 +617,7 @@ DEPENDENCIES
   observer
   omniauth-google-oauth2
   omniauth-rails_csrf_protection
+  openssl
   ostruct
   parallel
   puma

app/controllers/api/v1/study_files_controller.rb

@@ -307,8 +307,18 @@ def perform_update!(study_file)
           study_file.build_ann_data_file_info if study_file.ann_data_file_info.blank?
 
           safe_file_params = study_file.ann_data_file_info.merge_form_data(study_file_params.to_unsafe_hash)
-          # override to enforce metadata convention for all AnnData parsing
-          safe_file_params[:use_metadata_convention] = true
+          # use feature flag state for metadata convention, checking if either study or user has value configured
+          # otherwise default to true for backwards compatibility
+          convention_flag = FeatureFlag.find_by(name: 'convention_required')
+          flag_configured = study.flag_configured?('convention_required') ||
+                            current_api_user.flag_configured?('convention_required')
+          if convention_flag && flag_configured
+            safe_file_params[:use_metadata_convention] = FeatureFlaggable.merged_value_for(
+              convention_flag.name, current_api_user, study
+            )
+          else
+            safe_file_params[:use_metadata_convention] = true
+          end
         else
           safe_file_params = study_file_params
         end

app/javascript/components/upload/AnnDataFileForm.jsx

@@ -15,7 +15,8 @@ export default function AnnDataFileForm({
   deleteFile,
   bucketName,
   isInitiallyExpanded,
-  isAnnDataExperience
+  isAnnDataExperience,
+  conventionRequired
 }) {
 
   const validationMessages = validateFile({
@@ -24,7 +25,7 @@ export default function AnnDataFileForm({
 
   return <ExpandableFileForm {...{
     file, allFiles, updateFile, saveFile,
-    allowedFileExts, deleteFile, validationMessages, bucketName, isInitiallyExpanded, isAnnDataExperience
+    allowedFileExts, deleteFile, validationMessages, bucketName, isInitiallyExpanded, isAnnDataExperience, conventionRequired
   }}>
     <TextFormField label="Description" fieldName="description" file={file} updateFile={updateFile}/>
   </ExpandableFileForm>

app/javascript/components/upload/AnnDataUploadStep.jsx

@@ -18,6 +18,7 @@ export default {
 
 /** Renders a form for uploading one or more AnnData files */
 function AnnDataUploadStep({
+  serverState,
   formState,
   addNewFile,
   updateFile,
@@ -27,6 +28,8 @@ function AnnDataUploadStep({
   deleteFileFromForm
 }) {
   const AnnDataFile = formState.files.filter(AnnDataFileFilter)
+  const featureFlagState = serverState.feature_flags
+  const conventionRequired = featureFlagState && featureFlagState.convention_required
   useEffect(() => {
     if (AnnDataFile.length === 0) {
       addNewFile(DEFAULT_NEW_ANNDATA_FILE)
@@ -55,7 +58,8 @@ function AnnDataUploadStep({
         annDataFileTypes={['AnnData']}
         bucketName={formState.study.bucket_id}
         isInitiallyExpanded={true}
-        isAnnDataExperience={isAnnDataExperience}/>
+        isAnnDataExperience={isAnnDataExperience}
+        conventionRequired={conventionRequired}/>
     })}
   </div>
 }

app/javascript/components/upload/ExpandableFileForm.jsx

@@ -10,7 +10,8 @@ import Button from 'react-bootstrap/lib/Button'
 /** renders its children inside an expandable form with a header for file selection */
 export default function ExpandableFileForm({
   file, allFiles, updateFile, allowedFileExts, validationMessages, bucketName,
-  saveFile, deleteFile, isInitiallyExpanded, isAnnDataExperience, isLastClustering = false, children
+  saveFile, deleteFile, isInitiallyExpanded, isAnnDataExperience, isLastClustering = false,
+  conventionRequired = false, children
 }) {
   const [expanded, setExpanded] = useState(isInitiallyExpanded || file.status === 'new')
 
@@ -52,7 +53,8 @@ export default function ExpandableFileForm({
               allowedFileExts={allowedFileExts}
               validationMessages={validationMessages}
               bucketName={bucketName}
-              isAnnDataExperience={isAnnDataExperience} />
+              isAnnDataExperience={isAnnDataExperience}
+              conventionRequired={conventionRequired}/>
           </div>}
           <SaveDeleteButtons {...{
             file, updateFile, saveFile, deleteFile, validationMessages, isAnnDataExperience, allFiles, isLastClustering

app/javascript/components/upload/FileUploadControl.jsx

@@ -19,7 +19,7 @@ export default function FileUploadControl({
   file, allFiles, updateFile,
   allowedFileExts=['*'],
   validationIssues={},
-  bucketName, isAnnDataExperience
+  bucketName, isAnnDataExperience, conventionRequired
 }) {
   const [fileValidation, setFileValidation] = useState({
     validating: false, issues: {}, fileName: null
@@ -72,7 +72,9 @@ export default function FileUploadControl({
     }
 
     setFileValidation({ validating: true, issues: {}, fileName: selectedFile.name })
-    const [issues, notes] = await ValidateFile.validateLocalFile(selectedFile, file, allFiles, allowedFileExts, isAnnDataExperience)
+    const [issues, notes] = await ValidateFile.validateLocalFile(
+      selectedFile, file, allFiles, allowedFileExts, isAnnDataExperience, conventionRequired
+    )
     setFileValidation({ validating: false, issues, fileName: selectedFile.name, notes })
     if (issues.errors.length === 0) {
       updateFile(file._id, {
@@ -133,7 +135,7 @@ export default function FileUploadControl({
     setFileValidation({ validating: true, issues: {}, fileName: trimmedPath })
     try {
       const issues = await ValidateFile.validateRemoteFile(
-        bucketName, trimmedPath, fileType, fileOptions, isAnnDataExperience
+        bucketName, trimmedPath, fileType, fileOptions, isAnnDataExperience, conventionRequired
       )
       setFileValidation({ validating: false, issues, fileName: trimmedPath })
 

app/javascript/lib/validation/validate-anndata.js

@@ -273,12 +273,19 @@ async function validateOntologyIdFormat(hdf5File) {
 }
 
 /** Parse AnnData file, and return an array of issues, along with file parsing info */
-export async function parseAnnDataFile(fileOrUrl, remoteProps) {
+export async function parseAnnDataFile(fileOrUrl, remoteProps, conventionRequired) {
   let issues = []
 
   const hdf5File = await getHdf5File(fileOrUrl, remoteProps)
 
   const headers = await getAnnDataHeaders(hdf5File)
+  const uniqueIssues = validateUnique(headers)
+  const alphanumericAndUnderscoreIssues = validateAlphanumericAndUnderscores(headers)
+  // if convention bypass is granted, do not validate anything else
+  if (!conventionRequired) {
+    issues = issues.concat(uniqueIssues, alphanumericAndUnderscoreIssues)
+    return { issues }
+  }
 
   const requiredMetadataIssues = validateRequiredMetadataColumns([headers], true)
   let ontologyIdFormatIssues = []
@@ -296,8 +303,8 @@ export async function parseAnnDataFile(fileOrUrl, remoteProps) {
   }
 
   issues = issues.concat(
-    validateUnique(headers),
-    validateAlphanumericAndUnderscores(headers),
+    uniqueIssues,
+    alphanumericAndUnderscoreIssues,
     requiredMetadataIssues,
     ontologyIdFormatIssues,
     ontologyLabelAndIdIssues

app/javascript/lib/validation/validate-file-content.js

@@ -418,7 +418,7 @@ export async function validateGzipEncoding(file, fileType) {
  */
 async function parseFile(
   file, fileType, fileOptions={},
-  sizeProps={}, remoteProps={}, isAnnDataExperience
+  sizeProps={}, remoteProps={}, isAnnDataExperience, conventionRequired
 ) {
   const startTime = performance.now()
 
@@ -469,7 +469,7 @@ async function parseFile(
     }
 
     if (fileType === 'AnnData') {
-      const { issues } = await parseAnnDataFile(file, remoteProps)
+      const { issues } = await parseAnnDataFile(file, remoteProps, conventionRequired)
       parseResult.issues = parseResult.issues.concat(issues)
     } else if (parseFunctions[fileType]) {
       let ignoreLastLine = false

app/javascript/lib/validation/validate-file.js

@@ -50,9 +50,10 @@ function validateFileName(file, studyFile, allStudyFiles, allowedFileExts=['*'])
  * @param allStudyFiles {StudyFile[]} the array of all files for the study, used for name uniqueness checks
  * @param allowedFileExts { String[] } array of allowable extensions, ['*'] for all
  * @param isAnnDataExperience {Boolean} controls finding nested AnnData data fragments
+ * @param conventionRequired {Boolean} controls metadata convention validation
  */
 async function validateLocalFile(
-  file, studyFile, allStudyFiles=[], allowedFileExts=['*'], isAnnDataExperience
+  file, studyFile, allStudyFiles=[], allowedFileExts=['*'], isAnnDataExperience, conventionRequired
 ) {
   // if clientside file validation feature flag is false skip validation
   const flags = getFeatureFlagsWithDefaults()
@@ -73,7 +74,9 @@ async function validateLocalFile(
       use_metadata_convention: studyFile.use_metadata_convention
     }
     const { fileInfo, issues, perfTime, notes } =
-      await ValidateFileContent.parseFile(file, studyFileType, fileOptions,  {}, {}, isAnnDataExperience)
+      await ValidateFileContent.parseFile(
+        file, studyFileType, fileOptions,  {}, {}, isAnnDataExperience, conventionRequired
+      )
     const allIssues = issues.concat(nameIssues)
     issuesObj = formatIssues(allIssues)
     notesObj = notes
@@ -140,14 +143,14 @@ function getSizeProps(contentRange, contentLength, file) {
 *  @param {String} fileType SCP file type
 *  @param {Object} [fileOptions]
 *  @param isAnnDataExperience {Boolean} controls finding nested AnnData data fragments
-*
+*  @param conventionRequired {Boolean} controls metadata convention validation
 * @return {Object} issueObj Validation results, where:
 *   - `errors` is an array of errors,
 *   - `warnings` is an array of warnings, and
 *   - `summary` is a message like "Your file had 2 errors"
 */
 async function validateRemoteFile(
-  bucketName, fileName, fileType, fileOptions, isAnnDataExperience
+  bucketName, fileName, fileType, fileOptions, isAnnDataExperience, conventionRequired
 ) {
   const startTime = performance.now()
 
@@ -182,7 +185,7 @@ async function validateRemoteFile(
 
     // Equivalent block exists in validateFileContent
     const parseResults = await ValidateFileContent.parseFile(
-      file, fileType, fileOptions, sizeProps, remoteProps, isAnnDataExperience
+      file, fileType, fileOptions, sizeProps, remoteProps, isAnnDataExperience, conventionRequired
     )
     fileInfo = parseResults['fileInfo']
     issues = parseResults['issues']

app/models/ann_data_file_info.rb

@@ -30,7 +30,7 @@ class AnnDataFileInfo
   field :has_metadata, type: Boolean, default: false
   field :has_raw_counts, type: Boolean, default: false
   field :has_expression, type: Boolean, default: false
-  # controls whether or not to ingest data (true: should not ingest data, this is like an 'Other' file)
+  # controls whether to ingest data (true: should not ingest data, this is like an 'Other' file)
   field :reference_file, type: Boolean, default: true
   # location of raw count data, either .raw attribute or in layers[{name}]
   field :raw_location, type: String, default: ''

app/models/study.rb

@@ -742,6 +742,9 @@ def author
   validates_uniqueness_of :external_identifier, allow_blank: true
   validates :cell_count, numericality: { only_integer: true, greater_than_or_equal_to: 0 }
   validate :enforce_embargo_max_length
+  validate :check_user_org_on_private
+
+  ###
 
   # callbacks
   before_validation :set_url_safe_name
@@ -2009,6 +2012,16 @@ def enforce_embargo_max_length
     end
   end
 
+  def check_user_org_on_private
+    return true if public
+
+    if public_changed? && !public && user.organizational_email.blank?
+      errors.add(:base,
+                 'You must have an organizational email associated with your account to create private studies. ' \
+                   'Please update your organizational information in your profile settings.')
+    end
+  end
+
   # automatically create a FireCloud workspace on study creation after validating name & url_safe_name
   # will raise validation errors if creation, bucket or ACL assignment fail for any reason and deletes workspace on validation fail
   def initialize_with_new_workspace

image-pipeline/yarn.lock

@@ -1993,9 +1993,9 @@ lodash.merge@^4.6.0:
   integrity sha512-0KpjqXRVvrYyCsX1swR/XTK0va6VQkQM6MNo7PqW77ByjAhoARA8EfrP1N4+KlKj8YS0ZUCtRT/YUuhyYDujIQ==
 
 lodash@^4.17.14, lodash@^4.17.15, lodash@^4.17.19:
-  version "4.17.21"
-  resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.21.tgz#679591c564c3bffaae8454cf0b3df370c3d6911c"
-  integrity sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==
+  version "4.17.23"
+  resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.23.tgz#f113b0378386103be4f6893388c73d0bde7f2c5a"
+  integrity sha512-LgVTMpQtIopCi79SJeDiP0TfWi5CNEc/L/aRdTh3yIvmZXTnheWpKjSZhnvMl8iXbC1tFg9gdHHDMLoV7CnG+w==
 
 loglevel-colored-level-prefix@^1.0.0:
   version "1.0.0"

test/factories/user.rb

@@ -8,9 +8,11 @@
       # If specified, the created object will be added to the passed-in array after creation
       # this enables easy managing of a central list of objects to be cleaned up by a test suite
       test_array { nil }
+      sequence(:username) { |n| "test.user.#{n}" }
     end
     # https://github.com/thoughtbot/factory_bot/blob/main/GETTING_STARTED.md#sequences
-    sequence(:email) { |n| "test.user.#{n}@test.edu" }
+    email { "#{username}@test.edu" }
+    organizational_email { "#{username}@test.edu" }
     uid { rand(10000..99999) }
     password { "test_password" }
     metrics_uuid { SecureRandom.uuid }

test/js/lib/validate-anndata.test.js

@@ -9,6 +9,7 @@ import {
 } from './node-web-api'
 
 const BASE_URL = 'https://github.com/broadinstitute/single_cell_portal_core/raw/development/test/test_data/anndata'
+const CONVENTION_REQUIRED = true
 
 describe('Client-side file validation for AnnData', () => {
   beforeAll(() => {
@@ -38,20 +39,20 @@ describe('Client-side file validation for AnnData', () => {
       'species__ontology_label'
     ]
     const remoteProps = { url }
-    const hdf5File = await getHdf5File(url, remoteProps)
+    const hdf5File = await getHdf5File(url, remoteProps, CONVENTION_REQUIRED)
     const headers = await getAnnDataHeaders(hdf5File)
     expect(headers).toEqual(expectedHeaders)
   })
 
   it('Reports AnnData with valid headers as valid', async () => {
     const url = `${BASE_URL}/valid.h5ad`
-    const parseResults = await parseAnnDataFile(url)
+    const parseResults = await parseAnnDataFile(url, {}, CONVENTION_REQUIRED)
     expect(parseResults.issues).toHaveLength(0)
   })
 
   it('Reports AnnData with invalid headers as invalid', async () => {
     const url = `${BASE_URL}/invalid_header_no_species.h5ad`
-    const parseResults = await parseAnnDataFile(url)
+    const parseResults = await parseAnnDataFile(url, {}, CONVENTION_REQUIRED)
 
     expect(parseResults.issues).toHaveLength(1)
 
@@ -82,7 +83,7 @@ describe('Client-side file validation for AnnData', () => {
 
   it('Parses AnnData rows and reports invalid ontology IDs', async () => {
     const url = `${BASE_URL}/invalid_disease_id.h5ad`
-    const parseResults = await parseAnnDataFile(url)
+    const parseResults = await parseAnnDataFile(url, {}, CONVENTION_REQUIRED)
 
     expect(parseResults.issues).toHaveLength(1)
 

test/models/study_test.rb

@@ -37,6 +37,10 @@ class StudyTest < ActiveSupport::TestCase
     @services_args = [String, String, String]
   end
 
+  teardown do
+    @user.update(organizational_email: @user.email)
+  end
+
   after(:all) do
     Study.where(firecloud_workspace: 'bucket-read-check-test').delete_all
   end
@@ -281,4 +285,19 @@ class StudyTest < ActiveSupport::TestCase
     study.reload
     assert_equal [new_name, new_cluster.name], study.default_cluster_order
   end
+
+  test 'should enforce organizational email requirement' do
+    @study.public = false
+    assert @study.valid?
+
+    @user.update(organizational_email: nil)
+    @study.public = true
+    assert @study.valid?
+
+    @study.public = false
+    assert_not @study.valid?
+    assert @study.errors.messages_for(:base).first.starts_with?(
+      'You must have an organizational email associated with your account'
+    )
+  end
 end

yarn.lock

@@ -6689,9 +6689,9 @@ lodash.sortby@^4.7.0:
   integrity sha512-HDWXG8isMntAyRF5vZ7xKuEvOhT4AhlRt/3czTSjvGUxjYCBVRQY48ViDHyfYz9VIoBkW4TMGQNapx+l3RUwdA==
 
 lodash@^4.17.14, lodash@^4.17.15, lodash@^4.17.19, lodash@^4.17.21, lodash@^4.17.4, lodash@^4.7.0:
-  version "4.17.21"
-  resolved "https://registry.npmjs.org/lodash/-/lodash-4.17.21.tgz"
-  integrity sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==
+  version "4.17.23"
+  resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.23.tgz#f113b0378386103be4f6893388c73d0bde7f2c5a"
+  integrity sha512-LgVTMpQtIopCi79SJeDiP0TfWi5CNEc/L/aRdTh3yIvmZXTnheWpKjSZhnvMl8iXbC1tFg9gdHHDMLoV7CnG+w==
 
 loglevel-colored-level-prefix@^1.0.0:
   version "1.0.0"