EditBug:10100 Email and website fields in author update interleaved with semicolons

From ISFDB
Jump to navigation Jump to search

I tracked this down in the source; the problem is that PrintMultField() expects a sequence as its last argument, but the viewer is passing the plain ol' string (containing a semicolon-separated list of email addresses). Python will treat a string as a sequence of characters, though, so PrintMultiField() interprets it as a list of 1-character email addresses, and lists them all, separated by semicolons.

I started fixing the problem in the code, but realized that if I wanted to test my changes I'd need to install a whole mysql system with some test data and that's more trouble than I want to go to right now. However, I did notice that if you change the author_emails variable to be a tuple (or list) of strings, instead of a single string containing semicolon-separated values, some of the code gets noticeably simpler. And you can get rid of the used_emails variable since it's 1 iff (author_emails is not the empty tuple), which is equivalent to just evaluating the author_emails tuple as a boolean.

Here's an rcsdiff of what I did before stopping --- if you decide to go this route you presumably need to grep for the remaining references to used_emails and author_emails and change them:

RCS file: common/RCS/authorClass.py,v
retrieving revision 1.5
diff -r1.5 common/authorClass.py
31d30
< 		self.used_emails = 0
42c41
< 		self.author_emails = ''
---
> 		self.author_emails = ()
85,94c84
< 			rec2 = res2.fetch_row()
< 			if rec2:
< 				self.used_emails = 1
< 				self.author_emails = ""
< 				while rec2:
< 					if self.author_emails == "":
< 						self.author_emails = rec2[0][0]
< 					else:
< 						self.author_emails += ";" + rec2[0][0]
<                         		rec2 = res2.fetch_row()
---
> 			self.author_emails = tuple(map(lambda x: x[0], res2.fetch_row(0)))
146c136
< 			if self.used_emails:
---
> 			if len(self.author_emails):
148c138
< 						(self.author_emails)
---
> 						(';'.join(self.author_emails))
214,215c204
< 			self.used_emails = 1
< 			self.author_emails = elem
---
> 			self.author_emails = tuple(elem.split(';'))
253a243
> 		emails = [ ]
256,260c246
< 			if counter == 1:
< 				self.author_emails = email
< 				self.used_emails = 1
< 			else:
< 				self.author_emails += ';'+email
---
> 			emails.append(email)
261a248
> 		self.author_emails = tuple(emails)
===================================================================
RCS file: edit/RCS/submitauth.py,v
retrieving revision 1.6
diff -r1.6 edit/submitauth.py
155,164c155
< 	update = 0
< 	if new.used_emails and old.used_emails:
< 		if new.author_emails != old.author_emails:
< 			update = 1
< 	elif new.used_emails and (old.used_emails == 0):
< 		update = 1
< 	elif (new.used_emails == 0) and old.used_emails:
< 		new.author_emails = ""
< 		update = 1
< 	if update:
---
> 	if new.author_emails != old.author_emails:
166,167c157
< 		emails = string.split(new.author_emails, ';')
< 		for email in emails:
---
> 		for email in new.author_emails:
===================================================================
Note that there is a workaround, though it's clumsy; if you delete the corrupted info and re-add it, it works fine. It's only when the data already existed on the record that it is corrupted. Mike Christie (talk) 22:05, 3 Apr 2007 (CDT)


What you've proposed is a better solution than what I've just implemented, but here's my rationale for doing something different in the short term. There are four field types that utilize PrintMultField(): emails, webpages, authors, and artists. Technically all four should be implemented the same way. Authors and artists currently use an array, while emails and webpages use concatenated strings (which is where the problem arises in the first place). I like the idea of using tuples, but that should be applied across all of the data types, including authors and artists. That would fix some other problems with authors and artists (such as when there are too many of them in a particular work). Doing it the right way would require a lot of changes, which I'm not exactly up for today.
So the completely wimp out way I've implemented was to change the viewer for DisplayAuthorChanges() from:
   PrintMultField('Emails',   'Email',   ';', doc, merge, current.used_emails, current.author_emails)
   PrintMultField('Webpages', 'Webpage', ';', doc, merge, current.used_webpages, current.author_webpages)
To:
   tmp = string.split(current.author_emails, ';')
   PrintMultField('Emails',   'Email',   ';', doc, merge, current.used_emails, tmp)
   tmp = string.split(current.author_webpages, ';')
   PrintMultField('Webpages', 'Webpage', ';', doc, merge, current.used_webpages, tmp)
which should get us by in the short term. Alvonruff 07:22, 14 Apr 2007 (CDT)