Edgewall Software
Modify

Opened 6 years ago

Closed 6 years ago

Last modified 11 months ago

#12338 closed defect (fixed)

NameError: global name 'datetime' is not defined

Reported by: Christian Boos Owned by: Jun Omae
Priority: normal Milestone: 1.2
Component: ticket system Version: 1.2dev
Severity: normal Keywords:
Cc: leho@… Branch:
Release Notes:

Fix missing datetime import and crashing with invalid datetime string for time custom fields.

API Changes:
Internal Changes:

Description

How to Reproduce

While doing a POST operation on /newticket, Trac issued an internal error.

When creating a ticket on trunk, got the following traceback (I have some custom fields defined):

Python Traceback

Traceback (most recent call last):
  File "D:\Trac\repos\trunk\trac\web\main.py", line 607, in _dispatch_request
    dispatcher.dispatch(req)
  File "D:\Trac\repos\trunk\trac\web\main.py", line 256, in dispatch
    resp = chosen_handler.process_request(req)
  File "D:\Trac\repos\trunk\trac\ticket\web_ui.py", line 178, in process_request
    return self._process_newticket_request(req)
  File "D:\Trac\repos\trunk\trac\ticket\web_ui.py", line 521, in _process_newticket_request
    valid = valid and self._validate_ticket(req, ticket)
  File "D:\Trac\repos\trunk\trac\ticket\web_ui.py", line 1367, in _validate_ticket
    isinstance(value, datetime)):
NameError: global name 'datetime' is not defined

A regression from 14481#file23.

Attachments (0)

Change History (9)

comment:1 by Jun Omae, 6 years ago

Owner: set to Jun Omae
Status: newassigned

Thanks for the catching. I'm going to fix it.

comment:2 by Jun Omae, 6 years ago

Currently, missing unit tests for time fields. I noticed another issue while adding the unit tests.

Posting invalid datetime for the time fields leads the following error.

  1. Add verified_at = time to [ticket-custom] section.
  2. Visit /newticket?verified_at=invalid
File "/src/tracdev/git/trac/web/main.py", line 607, in _dispatch_request
  dispatcher.dispatch(req)
File "/src/tracdev/git/trac/web/main.py", line 256, in dispatch
  resp = chosen_handler.process_request(req)
File "/src/tracdev/git/trac/ticket/web_ui.py", line 179, in process_request
  return self._process_newticket_request(req)
File "/src/tracdev/git/trac/ticket/web_ui.py", line 544, in _process_newticket_request
  fields = self._prepare_fields(req, ticket)
File "/src/tracdev/git/trac/ticket/web_ui.py", line 1654, in _prepare_fields
  format, value) if value else ''
File "/src/tracdev/git/trac/util/datefmt.py", line 895, in user_time
  return func(*args, **kwargs)
File "/src/tracdev/git/trac/util/datefmt.py", line 901, in format_date_or_datetime
  return format_datetime(*args, **kwargs)
File "/src/tracdev/git/trac/util/datefmt.py", line 305, in format_datetime
  return _format_datetime(t, format, tzinfo, locale, 'datetime')
File "/src/tracdev/git/trac/util/datefmt.py", line 261, in _format_datetime
  t = to_datetime(t, tzinfo or localtz)
File "/src/tracdev/git/trac/util/datefmt.py", line 172, in to_datetime
  type(t))

comment:3 by Christian Boos, 6 years ago

If we want to ensure having a valid value, we could introduce an as_datetime(userinput, default=None) where None would mean current time, similar to the as_int, as_bool we already have.

Most of the to_* functions can raise an exception for invalid input, only a few take anything (e.g. to_unicode).

comment:4 by Jun Omae, 6 years ago

Adding as_datetime sounds a good idea and useful. However, it cannot be used for this issue. I think we should show warning for such an invalid input.

Proposed changes to fix and add unit tests in log:jomae.git@t12338.

comment:5 by Jun Omae, 6 years ago

If a ticket has invalid value for time field, ticket[timefield] is None. No way to retrieve original value. Therefore, we cannot show the invalid value for input element for the time field. If an invalid value is submitted, we cannot show the submitted value even though we show warning for the value.

Should we show the invalid value in the input element of time field?

comment:6 by Christian Boos, 6 years ago

I haven't looked at the specifics and why the original value is gone (even not present in the req.args anymore?), but maybe we should rather move the precise validation client side.

Can't we somehow reuse the datepicker to validate the content of the field before sending it, even the value has been entered by hand (parseDate)?

If this check is bypassed, then we can reject the change without giving much details.

comment:7 by lkraav <leho@…>, 6 years ago

Cc: leho@… added

in reply to:  6 comment:8 by Jun Omae, 6 years ago

Missing import has been fixed and added unit tests for time fields in [14514].

Replying to Christian Boos:

I haven't looked at the specifics and why the original value is gone (even not present in the req.args anymore?), but maybe we should rather move the precise validation client side.

Thanks for the reply. I try to improve handling for the time fields, log:jomae.git@t12338.1.

When invalid datetime string is submitted via form or query string, the original string would be shown in the input element and warning be would be shown.

comment:9 by Jun Omae, 6 years ago

Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

Fixed in [14515].

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Jun Omae.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Jun Omae to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.