Merge lp://qastaging/~rick-rickspencer3/desktopcouch/notempviewsmods into lp://qastaging/desktopcouch

Proposed by Rick Spencer
Status: Merged
Approved by: Rick McBride
Approved revision: 28
Merged at revision: not available
Proposed branch: lp://qastaging/~rick-rickspencer3/desktopcouch/notempviewsmods
Merge into: lp://qastaging/desktopcouch
Diff against target: None lines
To merge this branch: bzr merge lp://qastaging/~rick-rickspencer3/desktopcouch/notempviewsmods
Reviewer Review Type Date Requested Status
Chad Miller (community) Approve
Stuart Langridge (community) Approve
Review via email: mp+9698@code.qastaging.launchpad.net

Commit message

There are two changes here:

Change 1 you probably care about. I refactored server.py by renaming server.get_records_and_type to server.get_records. Also modified the function to take a record type param if desired to only create records of that type and optionally create a view name "get_"+ record_type.

Change 2 changed couchwidget to use the modified records api.

To post a comment you must log in.
Revision history for this message
Rick Spencer (rick-rickspencer3) wrote :

There are two changes here:

Change 1 you probably care about. I refactored server.py by renaming server.get_records_and_type to server.get_records. Also modified the function to take a record type param if desired to only create records of that type and optionally create a view name "get_"+ record_type.

Change 2 changed couchwidget to use the modified records api.

Revision history for this message
Stuart Langridge (sil) wrote :

Looks good to me. I'd like to see a review from Chad as well since he built the code that's changing.

review: Approve
Revision history for this message
Chad Miller (cmiller) wrote :

I don't think we want to push a specific record type in a general function that we store. Consider:

rows = get_records(self, record_type="foo", create_view=True)
rows = get_records(self, record_type=" BAR", create_view=True)

The second call is going to give values of "foo", because it's stored in the design document from the first time.

You have to either remove storing the record type or name the view with the record_type.

review: Needs Fixing
Revision history for this message
Chad Miller (cmiller) wrote :

Additionally, I like having some annotation in the function name that suggests what index or slice operations take as values.

Revision history for this message
Chad Miller (cmiller) wrote :

I'm happy with the current version. Changing my review state.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'desktopcouch/records/couchwidget.py'
2--- desktopcouch/records/couchwidget.py 2009-07-24 16:54:41 +0000
3+++ desktopcouch/records/couchwidget.py 2009-08-05 14:46:06 +0000
4@@ -68,7 +68,7 @@
5 def headings(self, headings):
6 self.__headings = headings
7 if self.record_type != None:
8- self.set_record_type(self.record_type)
9+ self.__populate_treeview()(self.record_type)
10
11 @property
12 def editable(self):
13@@ -85,7 +85,7 @@
14
15 #refresh the treeview if possible
16 if self.record_type != None:
17- self.set_record_type(self.record_type)
18+ self.__populate_treeview()(self.record_type)
19
20 @property
21 def database(self):
22@@ -100,7 +100,7 @@
23 self.__db = CouchDatabase(db_name, create=True)
24
25 if self.record_type != None:
26- self.set_record_type(self.record_type)
27+ self.__populate_treeview()(self.record_type)
28
29 @property
30 def record_type(self):
31@@ -116,7 +116,9 @@
32
33 #store the record type string
34 self.__record_type = record_type
35+ self.__populate_treeview()
36
37+ def __populate_treeview(self):
38 #if the database is not set up, just return
39 if self.__db == None:
40 return
41@@ -125,8 +127,7 @@
42 self.__reset_model()
43
44 #retrieve the docs for the record_type, if any
45- filter = """function(doc) { if (doc.record_type == '%s') { emit(doc._id, doc); } }""" %record_type
46- results = self.__db.query(filter)
47+ results = self.__db.get_records(record_type=self.__record_type,create_view=True)
48
49 #if headings are already assigned, set up headings and columns
50 if self.headings != None:
51
52=== modified file 'desktopcouch/records/server.py'
53--- desktopcouch/records/server.py 2009-07-30 15:57:52 +0000
54+++ desktopcouch/records/server.py 2009-08-05 14:43:01 +0000
55@@ -25,7 +25,7 @@
56 from couchdb.client import ResourceNotFound, ResourceConflict
57 from couchdb.design import ViewDefinition
58 import desktopcouch
59-from desktopcouch.records.record import Record
60+from record import Record
61
62
63 #DEFAULT_DESIGN_DOCUMENT = "design"
64@@ -208,17 +208,19 @@
65 except KeyError:
66 return []
67
68- def get_records_and_type(self, create_view=False,
69+ def get_records(self, record_type=None, create_view=False,
70 design_doc=DEFAULT_DESIGN_DOCUMENT):
71 """A convenience function to get records. We optionally create a view
72 in the design document. C<create_view> may be True or False, and a
73 special value, None, is analogous to O_EXCL|O_CREAT .
74-
75- Use the view to return *all* records. If there is no view to use or we
76- insist on creating a new view and cannot, raise KeyError .
77-
78- Use index notation on the result to get rows with a particular record
79- type.
80+
81+ Set record_type to a string to retrieve records of only that
82+ specified type. Otherwise, usse the view to return *all* records.
83+ If there is no view to use or we insist on creating a new view
84+ and cannot, raise KeyError .
85+
86+ You can use index notation on the result to get rows with a
87+ particular record type.
88 =>> results = get_records_and_type()
89 =>> for foo_document in results["foo"]:
90 ... print foo_document
91@@ -227,8 +229,12 @@
92 =>> results = get_records_and_type()
93 =>> people = results[['Person']:['Person','ZZZZ']]
94 """
95- view_name = "get_records_and_type"
96- view_map_js = """function(doc) { emit(doc.record_type, doc) }"""
97+ if record_type == None:
98+ view_name = "get_records_and_type"
99+ view_map_js = """function(doc) { emit(doc.record_type, doc) }"""
100+ else:
101+ view_name = "get_" + record_type
102+ view_map_js = """function(doc) {if(doc.record_type == '%s') { emit(doc._id, doc); }}""" %record_type
103
104 if design_doc is None:
105 design_doc = view_name

Subscribers

People subscribed via source and target branches