Roundup Tracker - Issues

Issue 2550612

classification
Title: Error with date fields when other fields are invalid
Type: crash Severity: normal
Components: Web interface Versions: 1.4
process
Status: open Resolution: remind
Dependencies: Superseder: History breaks when props are inaccessible
View: 1169513
Assigned To: Nosy List: antmail, ber, jerrykan, rouilj
Priority: Keywords: patch

Created on 2009-12-08 05:19 by jerrykan, last changed 2017-04-24 00:03 by rouilj.

Files
File name Uploaded Description Edit Remove
roundup_traceback.txt jerrykan, 2009-12-08 05:19 Roundup Traceback
issue2550612_patch1.txt jerrykan, 2009-12-08 05:26
history_fix.patch jerrykan, 2012-03-09 00:51
Messages
msg3927 Author: [hidden] (jerrykan) Date: 2009-12-08 05:19
I seem to have found a bug bug relating to date fields when other fields
on a form a invalid.

How to (I believe) repoduce:
- Have a node type that has a form with a date field
- Have an auditor for the same node that checks certain values and
raises a 'ValueError' if there is some problem

- Create or edit the node (ensuring there is no existing value for the
date field)
- Enter a date into the date field
- Enter a value into another fieldf that will cause a 'ValueError'
- Submit the form

The process should fail because the template engine(?) treats the date
value as a 'Date', when instead it is still a 'String'. See attached
traceback.
msg3928 Author: [hidden] (jerrykan) Date: 2009-12-08 05:26
I have attached a potential fix/patch. I'm not sure this is the best way
to fix it, but it seems to do what I want.
msg4512 Author: [hidden] (jerrykan) Date: 2012-03-06 10:35
Turns out that this issue is not related to displaying the date using
field(), but attempting to display the date in the history section of
the issue. When generating the "current state" the DateHTMLProperty
._value contains a str instead of Date which causes problems when
calling .plain()

The actual issue is therefore: values that have not been committed to
the database (due to an auditor error) are showing up in this history
when an auditor error occurs.

To reproduce this subtle issue (in a visual way):
 * Create a new issue that is assigned to someone ("demo")
 * Edit the issue, and un-assign it from anyone ("- no selection -")
 * Edit the issue once again, and:
    * assign the issue to someone ("demo")
    * delete the contents of the title (to trigger an auditor error)

The history should look like:
  2012-03-06 09:28:06	demo	set	assignedto: demo -> 
  2012-03-06 09:27:54	demo	create	

But instead it looks like:
  2012-03-06 09:28:06	demo	set	assignedto: demo -> demo
  2012-03-06 09:27:54	demo	create
msg4513 Author: [hidden] (jerrykan) Date: 2012-03-09 00:51
A new patch that fixes the underlying history problem...

It checks the DB to see if the prop value exists before trying to fetch
it from the DB. This stops the value in the form from being fetched if
there is no value for the prop in the DB.

This probably isn't the most elegant solution, because it requires
hitting the DB twice if a prop value does exist in the DB (instead of
just once), but it is preferable to rewriting or duplicating a decent
amount of code.
msg5089 Author: [hidden] (ber) Date: 2014-04-25 13:00
----------Original Message----------
From: Anthony Pankov <ant_mail@inbox.ru>
Sent: Friday 25 April 2014, 14:09:11
To: "roundup-devel" <roundup-devel@lists.sourceforge.net>
Cc: 
Subject: [Roundup-devel] 4 years opened issue and templating.py 
restructuring

Hello, All.

There is an issue http://issues.roundup-tracker.org/issue2550612 which
i can confirm.

As  i  see,  incorrect  date value is raised as DateHTMLProperty class
after  post.  For  example,  '999'  entered in date field.  '999' will
appear  as  string  _value  in base  HTMLProperty class and date value will 
be
None      in     ancestor     DateHTMLProperty.     When    displaying
DateHTMLProperty.pretty(),  DateHTMLProperty.plain()
functions   check   whether   _value   is  None  and  if  not  try  to
self._value.local(offset)  which  raise  an  error. This is because of
_value is not None, it is incorrect date string.

Converting in templating.py DateHTMLProperty.pretty() function from:
       if not self._value:
             return ''
       elif format is not self._marker:
            return self._value.local(offset).pretty(format)
       else:
           return self._value.local(offset).pretty()
to:
       if not self._value:
             return ''
       elif isinstance(self._value, str):
             return self._value
       elif format is not self._marker:
            return self._value.local(offset).pretty(format)
       else:
           return self._value.local(offset).pretty()

solve issue for me.

But  this  additional check must be copy-pasted in 
DateHTMLProperty.plain()
function     and    other.    It   is   hard   to   force myself to do
it.

There  is  a  20  entry  of  "  if  not  self.is_view_ok():"  check in
templating.py.

Is  there any plan to restructuring templating.py. What about to place
some common functionality to base HTMLproperty class?




Or,  may be somebody suggest another way to solve issue.
msg5632 Author: [hidden] (rouilj) Date: 2016-06-26 19:11
I am confused.

Is this the same problem as: 

 issue1169513: History breaks when props are inaccessible

or something more? (Added the link to superseder to make
sure this possibly related issue isn't lost in the message history
although it's not really a superseder).

Using jerrykan's reproduction steps in msg4512
in the current demo.py classic tracker only produces:

Required issue property title not supplied 

as an error and the history looks like:

2016-06-26 19:04:20	admin	set	assignedto: admin ->
2016-06-26 19:03:54	admin	set	assignedto: admin
2016-06-26 19:03:49	admin	create

was this fixed and this ticket not closed?

Do I need to have a date field in the schema to see this issue?

If the problem is that an entry is added to the history
even though the entry was rolled back by an auditor error
that seems to be a different diagnosis.

This tracker is using sqlite back end. If the problem is a history
non-rollback maybe this only affects anydbm backends?

I would like to get rid of known crash bugs with patches in the 
next (1.6) release if possible.

-- rouilj
msg5932 Author: [hidden] (rouilj) Date: 2017-02-26 02:41
Jerry, can you try this again with a current head release.

I have a tracker with two date fields and an integer field.

If I put a valid date in one field and put a real number in
the integer field, an edit exception is raised. The date field
is still properly displayed on the form.

Same when putting the value 10 in my second date field. The first
date field is still ok and no sign of a crash.

If my test above looks valid and you can't reproduce or you are
sufficiently convinced that my test shows the problem is gone
let me know and we can close this.
msg5963 Author: [hidden] (rouilj) Date: 2017-04-24 00:03
Does anybody know if this issue can be closed because other
changes to the code have fixed it?
History
Date User Action Args
2017-04-24 00:03:17rouiljsetmessages: + msg5963
2017-02-26 16:28:37rouiljsetresolution: works for me -> remind
2017-02-26 02:41:03rouiljsetresolution: works for me
messages: + msg5932
2016-06-26 19:11:57rouiljsetstatus: new -> open
type: crash
superseder: History breaks when props are inaccessible
messages: + msg5632
nosy: + rouilj
2014-04-25 13:00:02bersetnosy: + ber, antmail
messages: + msg5089
2012-03-09 00:51:39jerrykansetfiles: + history_fix.patch
keywords: + patch
messages: + msg4513
2012-03-06 10:35:08jerrykansetmessages: + msg4512
2009-12-08 05:26:30jerrykansetfiles: + issue2550612_patch1.txt
messages: + msg3928
2009-12-08 05:19:59jerrykancreate