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

Issue #31 - Bug fixes #32

Conversation

reynoldtan
Copy link
Member

@reynoldtan reynoldtan commented Feb 7, 2018

  • Page not found page redirect error
    FIX: made the system generate the url instead of hard-coding it. This way, the module can correctly point to the right url when folder structure is different from dev version.
    TO TEST: Create a test project in your dev and add header, user and environment data file to it. After adding, delete each one of them. In all delete/remove operation, the page should reload itself and not redirect to a page not found.
    NOTE: This cannot be tested on the portal since there is no option to delete a project once you do testing to it.

  • Not suggesting a trait in stage 2 - describe trait
    FIX: remove leading and trailing space to the trait.
    TO TEST: Upload a spreadsheet file that contains a trait that is in another project. Click next to stage 2 where a field "Did you mean" will provide you an option of similar traits.

  • Extra spaces in column headers
    FIX: replaced any multiple spaces in column headers into single space.
    TO TEST: Upload a spreadsheet file with a column header with multiple spaces
    eg. Plant height (cm), Days till 10% of Plants have One Open Flower

  • Non-MS Excel spreadsheet
    FIX: Added a check to see if spreadsheet file matched MS Excel MIME information.
    TO TEST: Use the file attached.
    LibreOffice Spreadsheet.xlsx

@carolyncaron
Copy link
Member

@Jiu9Shen can you test the 4th bullet point, using both Reynold's provided sheet and one of your own?

@carolyncaron
Copy link
Member

carolyncaron commented Mar 1, 2018

Page not found redirect error

  • I was not able to reproduce the problem with deleting a user or header on dev/fresh before switching to this branch.
  • I was able to reproduce it when deleting an environmental data file, and confirmed that this fixes it.

Not suggesting a trait in stage 2 - describe trait

  • Appears to work as expected. Tried it with variable amount of whitespace in different areas within the column header.

Extra spaces in column headers

  • I found that it now works for preceeding and trailing whitespace, as well as whitespace in between words for new and existing traits.
  • However, I found 2 test cases that need to be addressed:
  1. When a trait is essential, any whitespace between words will cause validation in step 1 to fail.
    Ex: Planting Date gives a "The following Essential traits must exist: Planting Date" error.
  2. For new traits, when a certain unit is subject to validation and doesn't match what is expected, an error occurs in the 3rd step when I run the job. For example, I uploaded a sheet with "Harvest Date". I chose to use a suggested trait from another project: "harvest date (date)". My values were text, so I received the following error on my terminal:
    WD rawpheno: Uploading Phenoypic Data: Missing Plant Measurement Type (Header=harvest date). [error] ERROR (RAWPHENO): Uploading Phenoypic Data: Missing Plant Measurement Type (Header=harvest date). [site http://default] [TRIPAL ERROR] [RAWPHENO] Uploading Phenoypic Data: Missing Plant Measurement Type (Header=harvest date). WD rawpheno: [CODE 106] Failed to load phenoypic data (job 5759) [error] CRITICAL (RAWPHENO): [CODE 106] Failed to load phenoypic data (job 5759) [site http://default] [TRIPAL CRITICAL] [RAWPHENO] [CODE 106] Failed to load phenoypic data (job 5759) Drush command terminated abnormally due to an unrecoverable error.
    My hunch is that the issue has to do with missing the validation step in the 1st step since the module didn't know what unit to expect until the 3rd step.

Non-MS Excel spreadsheet

  • @Jiu9Shen confirmed that your test spreadsheets prompts an error because of no content.
  • However, other test cases that contain data did not prompt errors during validation when expected. :-(
  • I think this is a difficult problem for you to debug without easy access to Linux or LibreOffice. And, since it is not urgent, I suggest we create a separate issue for this bug and ask @Jiu9Shen to make an attempt at it once we are upgraded to Tripal 3.

@reynoldtan
Copy link
Member Author

Updated code and fixed the error. I made a small update to make the default unit to "text" in create/add new column header form.

@laceysanderson
Copy link
Member

Confirmed by @carolyncaron

I just retested "whitespace between words in column header" and unfortunately I'm still receiving the "Essential Traits are missing" error 😞 in step #1
Test-1-NoErrors internal header space.xlsx
screen shot 2018-04-06 at 1 53 44 pm

Copy link
Member

@laceysanderson laceysanderson left a comment

Choose a reason for hiding this comment

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

I just retested "whitespace between words in column header" and unfortunately I'm still receiving the "Essential Traits are missing" error 😞 in step #1

@reynoldtan
Copy link
Member Author

The extra space bug fix pertains only to new traits as indicated by the issue. I will create a separate issue for extra spaces in project-related traits (essential, subset, etc.).

@laceysanderson
Copy link
Member

Ok, I created #41 to follow up with the whitespace error. With the understanding that the whitespace bug will be fixed in another issue, we will consider this issue approved 🎉 🎉 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants