Skip to content

Analysis cart creation #1025

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

Closed
wants to merge 24 commits into from
Closed

Analysis cart creation #1025

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
90339fd
skeleton for demoing
squirrelo Mar 20, 2015
72267a0
add conformation to removing proc data
squirrelo Mar 25, 2015
10f077d
Merge branch 'master' of https://github.com/biocore/qiita into cart-a…
squirrelo Mar 28, 2015
f1ca570
add patch to create default analyses for all existing users
squirrelo Mar 28, 2015
e362c25
add default analysis on user creation
squirrelo Mar 28, 2015
cfe9ba2
streamline UI for cart
squirrelo Mar 30, 2015
28f8480
further refining IU
squirrelo Mar 30, 2015
f0c3f80
add default analyses using only SQL
squirrelo Mar 31, 2015
6d30936
changes to tests to reflect patch
squirrelo Mar 31, 2015
4f3756f
fix more tests, add analysis_workflow steps for carts
squirrelo Mar 31, 2015
b5bf4ed
update user private_analyses to ignore default cart
squirrelo Mar 31, 2015
68488c7
update test again to reflect change
squirrelo Mar 31, 2015
90b3de2
merge upstream/master
squirrelo Apr 1, 2015
bd3d513
Merge branch 'master' of https://github.com/biocore/qiita into cart-a…
squirrelo Apr 2, 2015
f5026f8
move default analysis pull to user object
squirrelo Apr 2, 2015
874632b
implement the default_analysis in qiita_pet
squirrelo Apr 2, 2015
ddf2ff4
more comments addressed
squirrelo Apr 2, 2015
1dc1440
pep8
squirrelo Apr 2, 2015
cf1123f
remove magic numbers from tests
squirrelo Apr 2, 2015
15c71dc
replace processed_date retriveal
squirrelo Apr 3, 2015
8e129e6
add info modal for proc data
squirrelo Apr 3, 2015
e784006
more UI changes
squirrelo Apr 3, 2015
d7b4778
couple small UI changes
squirrelo Apr 3, 2015
ba42c6a
use info glyph
squirrelo Apr 3, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
add default analysis on user creation
  • Loading branch information
@squirrelo
squirrelo committed Mar 28, 2015
commit e362c25e3e28f20a9c22c2b5e3ee99519909f05f
20 changes: 20 additions & 0 deletions qiita_db/analysis.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,26 @@ def _status_setter_checks(self, conn_handler):
if self.check_status({"public"}):
raise QiitaDBStatusError("Can't set status away from public!")

@classmethod
def get_user_default(cls, user):
Copy link
Contributor

Choose a reason for hiding this comment

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

Uhm... from my point of view, is usually better to ask the user: give me your default cart. It will be a instance method rather than a classmethod, which is already a signal of better design...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This allows you to return the actual object already instantiated, as opposed to just the ID and instantiating, saving a DB call. Do we care about the extra DB call?

Copy link
Contributor

Choose a reason for hiding this comment

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

You are not saving any DB call...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The check if ID exists on Analysis instantiation isn't needed in this case. That's the DB call I'm referring to.

Copy link
Contributor

Choose a reason for hiding this comment

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

That call is executed when you are doing cls(aid) (It is calling the init)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, didn't realize that. Sure, can make that change then.

"""Returns the analysis object being used as default sample holder

Parameters
----------
user : User object
The user to get the default for

Returns
-------
Analysis object
The analysis used as default
"""
conn_handler = SQLConnectionHandler()
aid = conn_handler.execute_fetchone(
"SELECT analysis_id FROM qiita.analysis WHERE "
"email = %s AND dflt = true", [user.id])[0]
return cls(aid)

@classmethod
def get_by_status(cls, status):
"""Returns analysis ids for all Analyses with given status
Expand Down
7 changes: 7 additions & 0 deletions qiita_db/test/test_user.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,13 @@ def test_create_user(self):
'email': 'new@test.bar'}
self._check_correct_info(obs, exp)

# make sure default analysis created
sql = ("SELECT email, name, description, dflt FROM qiita.analysis "
"WHERE email = 'new@test.bar'")
obs = self.conn_handler.execute_fetchall(sql)
exp = [['new@test.bar', 'new@test.bar-dflt', 'dflt', True]]
self.assertEqual(obs, exp)

def test_create_user_info(self):
user = User.create('new@test.bar', 'password', self.userinfo)
self.assertEqual(user.id, 'new@test.bar')
Expand Down
15 changes: 13 additions & 2 deletions qiita_db/user.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,10 +224,21 @@ def create(cls, email, password, info=None):
# for sql insertion
columns = info.keys()
values = [info[col] for col in columns]

queue = "add_user_%s" % email
conn_handler.create_queue(queue)
# crete user
sql = ("INSERT INTO qiita.%s (%s) VALUES (%s)" %
Copy link
Contributor

Choose a reason for hiding this comment

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

I know you have not touch this code, but we have been (more or less) consistently using format for this type of changes. I would appreciate if you can change that. Non blocking...

(cls._table, ','.join(columns), ','.join(['%s'] * len(values))))
conn_handler.execute(sql, values)
conn_handler.add_to_queue(queue, sql, values)
# create user default sample holder
sql = ("INSERT INTO qiita.analysis "
Copy link
Contributor

Choose a reason for hiding this comment

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

This is personal opinion but I think this highly improves code readability and even when failures happen they're easier to read on the command line. Instead of using quotes ("), I like to user triple quotes (""") and then align the SQL in a easy to read way. In this example:

sql = """INSERT INTO qiita.analysis
           (email, name, description, dflt, analysis_status_id)
         VALUES (%s, %s, %s, %s, 1)"""

The cool thing about this, is that if the query fails, it gets also formatted on the CLI. Also it is a more natural way of reading SQL as things align better (note the small indentation on the column names).

This is not blocking though...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can do this, but it is also something to put in contributing.md and needs to be consistently done across the entire codebase. Again, if that's agreeable I will make the change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, it's not on contributing so no worries if you don't want to change it. I'm doing it as I'm changing other parts of the code, but there is no documentation at this point.

"(email, name, description, dflt, analysis_status_id) "
"VALUES (%s, %s, %s, %s, 1)")
conn_handler.add_to_queue(queue, sql,
(email, '%s-dflt' % email, 'dflt', True))

conn_handler.execute_queue(queue)

return cls(email)

@classmethod
Expand Down
6 changes: 2 additions & 4 deletions qiita_pet/handlers/analysis_handlers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,10 +367,8 @@ def validate_absolute_path(self, root, absolute_path):

class SelectedSamplesHandler(BaseHandler):
def get(self):
seldata = {
1: {1: ['sample %d' % i for i in range(40)], 2: ['sample %d' % i for i in range(40)], 3: ['sample %d' % i for i in range(40)]}
}
self.render("analysis_selected.html", seldata=seldata, info={})
seldata = Analysis.get_user_queue(self.current_user).samples
self.render("analysis_selected.html", seldata=seldata)

def post(self):
pass