Opened 13 years ago
Last modified 3 years ago
#9740 new defect
Impossible to CC a username with spaces
| Reported by: | Owned by: | ||
|---|---|---|---|
| Priority: | normal | Milestone: | 1.6.1 |
| Component: | ticket system | Version: | 0.12-stable |
| Severity: | normal | Keywords: | username spaces cc fieldrefactoring |
| Cc: | Branch: | ||
| Release Notes: | |||
| API Changes: | |||
| Internal Changes: | |||
Description
It is impossible to CC a username with spaces in the username such as "Joe Bloggs", attempting to enter that treats as two different users Joe and Bloggs.
Suggest first attempt to establish that usernames separated by commas as known usernames before splitting by spaces for known usernames.
Attachments (3)
Change History (15)
comment:1 by , 13 years ago
| Milestone: | → unscheduled |
|---|
follow-up: 3 comment:2 by , 10 years ago
CC is sanitized with "," for some time now. Couldn't spaces be dropped as separarator completely? Would make code easier.
Otherwise quoting should be supported.
comment:3 by , 9 years ago
| Keywords: | fieldrefactoring added |
|---|---|
| Milestone: | unscheduled → next-major-releases |
Replying to dstoecker:
CC is sanitized with "," for some time now. Couldn't spaces be dropped as separarator completely? Would make code easier.
+1, but would probably need an upgrade step to be sure that all existing lists are normalized with ',' separator.
by , 3 years ago
| Attachment: | ticket_9740.patch added |
|---|
Patch to enable spaces in user names for the cc list. Includes changes to functional and unit tests
comment:7 by , 3 years ago
This patch enables spaces for user names in the cc field. Cc entries were saved in the database with , as separator for some time and the documentation says all entries have to be separated by , anyway.
As a sideeffect the contents of ticket fields with type list (for example the keywords field) is shown with comma as separator, too (property box at the top of tickets). That's only a rendering thing. No code was added to allow entries with spaces for such fields. This was done to keep the code changes to a minimum.
Because of the new rendering a functional test had to be adapted.
AFAICS this change doesn't lead to any regressions. The tickets #10828 and #8597 were tested without problems.
And to really top it off, this ticket just crossed the 10 years threshold: ThisTicketWasOpenedTenYearsAgo. So we are good to go ;-).
follow-up: 9 comment:8 by , 3 years ago
Just quickly looked at the patch, but I believe we'd need a database upgrade step to handle existing space-separated entries.
comment:9 by , 3 years ago
Replying to Ryan J Ollos:
Just quickly looked at the patch, but I believe we'd need a database upgrade step to handle existing space-separated entries.
I was thinking about that too. The comma was introduced with r3240 quite some time ago. Question is if there are untouched 15 years old tickets with a populated cc list…
by , 3 years ago
| Attachment: | ticket_9740_dbupgrade.patch added |
|---|
Database upgrade and test cases for previous patch. (Enabling spaces in user names for the cc list)
comment:10 by , 3 years ago
| Milestone: | next-major-releases → 1.5.4 |
|---|
Will look at for the upcoming release. Thanks!
by , 3 years ago
| Attachment: | ticket_9740-2.patch added |
|---|
Complete patch to enable spaces in user names for the cc list. Includes environment upgrade, changes to functional and unit tests and additional unit tests.
comment:11 by , 3 years ago
I added some unit tests for cc filed handling and created a single patch file.
Patch includes now:
- fix for spaces in usernames (same as first patch)
- changes to existing unit and functional tests to account for the fix (same as first patch)
- database upgrade to make sure cc field is properly formatted (same as second patch)
- additional unit tests for cc field handling (new)
comment:12 by , 3 years ago
| Milestone: | 1.5.4 → 1.6.1 |
|---|
If you can come up with a simple patch for the above suggestion, then why not, PatchWelcome. Otherwise I'm not too keen on making the CC: code more complex than it already is, when we're targeting an overhaul of the notification system (#1660).