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

[Image Uploader] fix naming convention check #2100

Merged
merged 3 commits into from Aug 16, 2016

Conversation

gluneau
Copy link
Contributor

@gluneau gluneau commented Aug 11, 2016

Previous name was too strict.

  • Could not upload a second scan set for same visit.
  • Could not include RES or HOS to separate scanners.

@gluneau gluneau added the Bug PR or issue introducing/requiring bug fixes (not mutually exclusive with the Feature label) label Aug 11, 2016
@gluneau gluneau added this to the 16.1 milestone Aug 11, 2016
@gluneau gluneau changed the title fix naming convention fix naming convention check Aug 11, 2016
if (($file_name != $pscid."_". $candid ."_". $visit_label . ".zip")
&& ($file_name != $pscid."_". $candid ."_". $visit_label . ".tgz")
&& ($file_name != $pscid."_". $candid ."_". $visit_label . ".tar.gz")
) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion:

/**
   * Checks if the inputted file name is valid
   *
   * @param {string} requiredFileName - Required file name
   * @param {string} fileName - Provided file name
   * @return {boolean} - true if file names match and false otherwise
   */
  isValidFileName: function(requiredFileName, fileName) {
    if (fileName === null || requiredFileName === null) {
      return false;
    }

    return (fileName.indexOf(requiredFileName) > -1);
  }

Note: snippet is in JS, but can be adapted to PHP easily using strpos()

Note 2: If you want requiredFileName to always be the beginning of the fileName, use:
return (fileName.indexOf(requiredFileName) === 0);

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alternatively you could use a regular expression. Something likelike:

if preg_match(/^{$pscid}{$candid}{$visit_label}.*(zip|tgz|tar.gz/) {
}

@gluneau gluneau changed the title fix naming convention check [Image Uploader] fix naming convention check Aug 11, 2016
@gluneau gluneau force-pushed the imaging_uploader_file_naming branch from 789fae4 to b8add23 Compare August 16, 2016 14:03
@codecov-io
Copy link

Current coverage is 13.71% (diff: 0.00%)

Merging #2100 into 16.1-dev will increase coverage by 0.27%

@@           16.1-dev      #2100   diff @@
==========================================
  Files           118        118          
  Lines         19923      20065   +142   
  Methods        1116       1116          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           2678       2752    +74   
- Misses        17245      17313    +68   
  Partials          0          0          

Sunburst

Powered by Codecov. Last update 10b611b...b8add23

@driusan driusan merged commit b5bbfb7 into aces:16.1-dev Aug 16, 2016
@christinerogers christinerogers added the Document at Release PR adds or changes a feature such that the wiki (or other documentation) must be updated label Aug 25, 2016
@christinerogers
Copy link
Contributor

Update documentation to tell people what filenames the imaging uploader will accept:

  • Wiki: Imaging Database Setup page
  • help text for the module

kongtiaowang pushed a commit to kongtiaowang/Loris that referenced this pull request Sep 9, 2016
* fix naming convention

* length of $candid & $visit_label can vary, looking for the whole string at once

* rephrase the error message
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug PR or issue introducing/requiring bug fixes (not mutually exclusive with the Feature label) Document at Release PR adds or changes a feature such that the wiki (or other documentation) must be updated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants