Skip to content
This repository has been archived by the owner on Sep 23, 2023. It is now read-only.

Update Barcode Scanner #632

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

teravest
Copy link
Contributor

Addresses issue #626

This updates the barcode scanner to be able to edit the fields in the barcode table.

I also created a controlled vocabulary for the sequencing_status column.

I also rewrote a giant req.write in barcode_utils.psp so that it was in html instead of a string.

@ElDeveloper
Copy link
Member

  • Make a header for american gut vs the name of the project.
  • For biomass remaining change checkbox to drop down.
  • In the edit existing kit in the american gut utilities remove the project drop down.
  • Make project group table that determines where the project belongs to.
  • Add a column to project
  • Add a column to project_barcode

@teravest
Copy link
Contributor Author

Ok all these changes should be made with the info we have right now.

@ElDeveloper
Copy link
Member

👍

@teravest
Copy link
Contributor Author

This is on webdev now :)

@adamrp
Copy link
Member

adamrp commented Feb 21, 2014

Thanks!

On Fri, Feb 21, 2014 at 4:22 PM, teravest [email protected] wrote:

This is on webdev now :)

Reply to this email directly or view it on GitHubhttps://github.com//pull/632#issuecomment-35784374
.

@adamrp
Copy link
Member

adamrp commented Feb 21, 2014

Minor aesthetic comment: can you change the <td> status rectangle (the thing that's green, red, purple, etc.) to have colspan=2 and align=center?

@adamrp
Copy link
Member

adamrp commented Feb 21, 2014

metadata checker gets triggered regardless of project and/or project_group, so that should be conditional for AG at this point

@adamrp
Copy link
Member

adamrp commented Feb 21, 2014

The default value for "Sequencing Status" in the database is NULL, so I think there needs to be an "unknown" (or "N/A" or something to that effect) option in that dropdown

@adamrp
Copy link
Member

adamrp commented Feb 22, 2014

Many of the fields have a "Current: " next to them (I assume so that you can tell if you're changing something before you hit submit?), but others don't.

@adamrp
Copy link
Member

adamrp commented Feb 22, 2014

Barcode 000007185 is office succession, but the checkbox or emailing says "Send kit owner an email" I might be mistaken, but I don't think that study has kits

@adamrp
Copy link
Member

adamrp commented Feb 22, 2014

Not sure, but I think only projects in the AG project group currently work (do not have access to SQL developer atm)

@teravest
Copy link
Contributor Author

I forgot the code to look up the barcode itself currently uses a stored procedure that looks in an american gut table. So this will only work with American Gut right now. I am going separate that into two stored procedures and only call the american gut one if it is an american gut barcode :)

@teravest
Copy link
Contributor Author

Updates are deployed on Webdev
Here is a list of barcodes to test with. One from each project
000000001
000006136
000005074
000007185
000005038
000007869
000010307
000011423
000006020

@teravest
Copy link
Contributor Author

teravest commented Mar 3, 2014

Updated webdev again this morning.

open group_name_ for
select g.group_name from project_groups g inner join project p on g.group_id = p.group_id
where p.project = project_name;
null;
Copy link
Member

Choose a reason for hiding this comment

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

what's the null for? Doesn't appear to be in other SPs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

uhg, I always forget to remove those. Its from the create new procedure thing on sql developer. I want to wait till tomorrow to do this so we don't lose it.

form['obsolete_status'])
data_access.setBarcodeProjType(form['project'], prev_barcode)
req.write("General Barcode Information Updated<br>")
except:
Copy link
Member

Choose a reason for hiding this comment

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

Can you except a particular type of Exception here?

Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you guys know what type of exceptions the database throws?

Copy link
Member

Choose a reason for hiding this comment

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

Not offhand, might have to check cx_Oracle docs

@ElDeveloper
Copy link
Member

@teravest not sure if this has to be part of this pull request, but I looked for barcode 000004595 and sent an e-mail, the e-mail came in as:

   In order to help you in adding these details, the side of the sample tube 000004595 reads:
      sample date: [Field('sample_date', ''), Field('sample_date', '')] 
      sample time: [Field('sample_time', ''), Field('sample_time', '')]

I'm looking at the code now.

con = self.getMetadataDatabaseConnection()
result = con.cursor()
con.cursor().callproc('get_sequencing_statuses', [result])
seq_statuses = [row[0] for row in result]
Copy link
Member

Choose a reason for hiding this comment

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

Can this fail with an IndexError and should this be caught?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think so. Even if no rows are returned seq_statuses will end up as an empty list. And unless the procedure returns a row with zero fields (I don't think this is possible), row[0] will always exist. This might be the kind of place you'd use an assert, I guess, because these tests would be for things that should not be possible

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha, thanks for the explanation.

@ElDeveloper
Copy link
Member

screen shot 2014-03-06 at 2 28 36 pm

@ElDeveloper
Copy link
Member

Great stuff @teravest

@adamrp
Copy link
Member

adamrp commented Mar 26, 2014

@teravest, is this ready to be merged?

@teravest
Copy link
Contributor Author

no, I have some things to change and its gonna need another review and more
testing :(

On Wed, Mar 26, 2014 at 11:07 AM, adamrp [email protected] wrote:

@teravest https://github.com/teravest, is this ready to be merged?

Reply to this email directly or view it on GitHubhttps://github.com//pull/632#issuecomment-38710888
.

@ElDeveloper
Copy link
Member

Fair enough, thanks @teravest

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

Successfully merging this pull request may close these issues.

3 participants