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.

Sign up for GitHub

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

Jump to bottom

Move selected to files qcstatus #1608

Merged
merged 7 commits into from
Mar 29, 2016
Merged

Move selected to files qcstatus #1608

merged 7 commits into from
Mar 29, 2016

Conversation

cmadjar
Copy link
Collaborator

@cmadjar cmadjar commented Mar 11, 2016

This pull request moves the selected information of the imaging browser into the files_qcstatus table.
Until now, this information was inserted into the parameter_file table and could be lost if need to reinsert the imaging data.

Cecile Madjar added 2 commits March 10, 2016 18:32
Commiting SQL patch needed to move the selected from the parameter_fi…
d8236c9 
…le table to the files_qc_status table
Move the selected information into the files_qcstatus table instead o…
048d358 
…f storing that information into the parameter_file table
@cmadjar cmadjar added Add to Release Notes PR change should be highlighted in Release notes (important security, features and bugfixes) Cleanup PR or issue introducing/requiring at least one clean-up operation Caveat for Existing Projects PR contains changes that might impact the code or accepted practices of current active projects SQL PR contains SQL modifications such as schema changes and new SQL scripts labels Mar 11, 2016
@cmadjar cmadjar added this to the 16.0 milestone Mar 11, 2016
@cmadjar cmadjar added the Discussion Required PR or issue awaiting the resolution of a discussion between all involved parties label Mar 11, 2016
@MounaSafiHarab
Copy link
Contributor

@cmadjar,

while you have an SQL patch for existing databases, I believe the schema should be updated as well when creating from scratch tables files_qcstatus and parameter_files.

@cmadjar
Copy link
Collaborator Author

cmadjar commented Mar 15, 2016

Very good point! I just added it to the Default schema. Thank you!!

@samirdas samirdas removed the Discussion Required PR or issue awaiting the resolution of a discussion between all involved parties label Mar 15, 2016
@codecov-io
Copy link

Current coverage is 14.66%

Merging #1608 into 16.04-dev will increase coverage by +0.01% as of 652eacd

@@            16.04-dev   #1608   diff @@
=========================================
  Files             112     112       
  Stmts           18832   18830     -2
  Branches            0       0       
  Methods          1046    1046       
=========================================
+ Hit              2760    2761     +1
  Partial             0       0       
+ Missed          16072   16069     -3

Review entire Coverage Diff as of 652eacd

Powered by Codecov. Updated on successful CI builds.

@MounaSafiHarab
Copy link
Contributor

Just tested this on my VM before and after applying the SQL patch. Info got copied properly into the added column, and further changes to the Image Panel in Imaging browser (namely the Selected field) resulted in the information being saved in the files_qcstatus table.

driusan
driusan reviewed Mar 18, 2016
View reviewed changes
SQL/2016-03-10-MoveSelectedTo_files_qcstatus_table.sql
@@ -0,0 +1,10 @@
/* ALTER files_qcstatus table to add a Selected column */
Copy link
Collaborator

Choose a reason for hiding this comment

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

These should be -- at the start of the line for a comment, not /* ... /. / is a php comment, -- is an SQL comment.

@cmadjar
Copy link
Collaborator Author

cmadjar commented Mar 18, 2016

This is fixed now. Thank you!

@driusan driusan mentioned this pull request Mar 23, 2016
Merged
@samirdas samirdas merged commit 471f2f9 into aces:16.04-dev Mar 29, 2016
@christinerogers
Copy link
Contributor

Does this have any impact on the DQT loading script or Statistics (Imaging subtab)?

@MounaSafiHarab MounaSafiHarab mentioned this pull request Mar 10, 2017
Merged
@cmadjar cmadjar deleted the MoveSelectedTo_files_qcstatus branch April 9, 2018 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@MounaSafiHarab MounaSafiHarab

Labels
Add to Release Notes PR change should be highlighted in Release notes (important security, features and bugfixes) Caveat for Existing Projects PR contains changes that might impact the code or accepted practices of current active projects Cleanup PR or issue introducing/requiring at least one clean-up operation SQL PR contains SQL modifications such as schema changes and new SQL scripts
Projects
None yet
Milestone
16.0
Development

Successfully merging this pull request may close these issues.
6 participants
@cmadjar @MounaSafiHarab @codecov-io @christinerogers @driusan @samirdas