Re: [Django Code] #7028: Better raw_id_fields feedback in newform-admins branch

[Django Code]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone:
Component: Admin interface | Version: newforms-admin
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Ready for checkin | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------------+------------------------
Comment (by julianb):

Can somebody have a look at #7923, which is related to that?

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:7>
Django Code <http://code.djangoproject.com/>
The web framework for perfectionists with deadlines

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone:

Component: django.contrib.admin | Version: newforms-admin

Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Ready for checkin | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------------+------------------------

Comment (by kmtracey):

#4134 was a dup, also with a patch.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:8>
Django <http://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone:

Component: django.contrib.admin | Version: 1.0

Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Ready for checkin | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------------+------------------------

Changes (by marc...@gmail.com):

* version: newforms-admin => 1.0

Comment:

Patch for 1.0

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:9>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone:
Component: django.contrib.admin | Version: 1.0
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Ready for checkin | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------------+------------------------

Comment (by anonymous):

There is an error, this line in admin_list.py:
result_name = repr(escape(force_unicode(result_id).replace("'",
'&prime')))[1:]
Should be:
result_name = repr(escape(force_unicode(result).replace("'",
'&prime')))[1:]

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:10>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone:
Component: django.contrib.admin | Version: 1.0
Resolution: | Keywords: raw-id-fields nfa-someday

Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 0 |
---------------------------------------------------+------------------------
Changes (by kmtracey):

* needs_tests: 0 => 1
* stage: Ready for checkin => Accepted

Comment:

The last comment makes this look not quite ready for checkin. Also there
are no tests in the latest patch, not even the earlier posted test changes
required to fix test breakage, so it isn't clear what exactly is supposed
to be applied. This ticket needs a consolidated patch with tests for the
new function before it can be called ready for checkin.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:11>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone:
Component: django.contrib.admin | Version: 1.0
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 0 |
---------------------------------------------------+------------------------

Comment (by paul...@gmail.com):

Line 182 of the patch to django/contrib/admin/widgets.py currently reads:
def label_for_value(self, value, nome=''):
but this is probably meant be:
def label_for_value(self, value, name=''):
This would not cause any tests to fail since the arg is not (currently)
used.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:12>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone:
Component: django.contrib.admin | Version: 1.0
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 0 |
---------------------------------------------------+------------------------
Comment (by paul...@gmail.com):

I just noticed that the empty string quotes after nome= and name= did not
get through to the last comment that I posted.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:13>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone:
Component: django.contrib.admin | Version: 1.0
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 0 |
---------------------------------------------------+------------------------

Comment (by kmtracey):

Replying to [comment:13 paul...@gmail.com]:

> I just noticed that the empty string quotes after nome= and name= did
not get through to the last comment that I posted.

They were interpreted as Wiki formatting characters which turned on, and
then off, italics. To avoid this you should mark code blocks as
preformatted text:
http://code.djangoproject.com/wiki/WikiFormatting#Preformattedtext. Also
checking how the comment will look via Preview before hitting Submit is
always a good idea.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:14>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody

Status: new | Milestone: 1.2

Component: django.contrib.admin | Version: 1.0
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 0 |
---------------------------------------------------+------------------------

Changes (by russellm):

* milestone: => 1.2

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:15>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: 1.0
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 0 |
---------------------------------------------------+------------------------

Comment (by mrts):

I've implemented this and also integrated #7923 in my !GitHub branch:
http://github.com/mrts/django/commits/ticket7028

Admin widget tests pass, but there's a regression in admin views tests --
a spurious `False` appears into the output, but I haven't yet got time to
figure out from where (probably a trivial problem somewhere).

Attaching a patch.

IMHO #7923 can be closed as duplicate.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:16>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: 1.0
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1

Needs_better_patch: 1 |
---------------------------------------------------+------------------------
Changes (by mrts):

* needs_better_patch: 0 => 1

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:17>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: 1.0
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 1 |
---------------------------------------------------+------------------------

Changes (by guettli):

* cc: h...@tbz-pariv.de (added)

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:18>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: 1.0
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 1 |
---------------------------------------------------+------------------------

Comment (by mrts):

Tests pass, looks more or less OK to me.

One thing that needs to be resolved is whether the object link should open
in a pop-up. That would be more consistent, but would make the
corresponding code horrible -- imagine embedding yet another `showPopup`
into the `\'`-quoted part:
{{{
' onclick="opener.dismissRelatedLookupPopup(window, %s, \'<a
href=&quot;%s&quot;>%s</a>\'); return false;"'
}}}

I'll probably implement this nevertheless.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:19>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2

Component: django.contrib.admin | Version: SVN

Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1

Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------------+------------------------
Changes (by mrts):

* needs_better_patch: 1 => 0
* version: 1.0 => SVN
* needs_tests: 1 => 0

Comment:

Tried my best to create a clean representation of the intent:
{{{
yield mark_safe(u'<%s%s><a href="%s"%s>%s</a></%s>' %
(table_tag, row_class, url,
(cl.is_popup
and ' onclick='

'"opener.dismissRelatedLookupPopup(window, %s,
'
"'<a href=&quot;%s&quot; "

'onclick=&quot;return
showRelatedObjectPopup(this);&quot;'
'>%s</a>\'); return false;"' %
(result_id, result_url, result_name)
or ''),
conditional_escape(result_repr), table_tag))
}}}

Commit:
http://github.com/mrts/django/commit/e95f27ad4b74eef3a9d4e9553e5cfb58f159c10a

Tests pass, also tested to be working in a real site.

Patch upcoming.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:20>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------------+------------------------

Comment (by mrts):

I'd say my work is completed. Any comments welcome.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:21>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------------+------------------------
Comment (by mrts):

I'd be much obliged if someone cared to test the
ticket7028-with_popup.patch in IE.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:22>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------------+------------------------

Comment (by marcob):

mrts, I tried it with IE7 but I got dreaded "Error on page". Can't find
where is the problem (I tried for at least on hour, sorry :-( IE is a pita
to debug). Hope there is someone smarter than me.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:23>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------------+------------------------

Comment (by mrts):

There are instructions for !JavaScript debugging in IE at
http://blogs.msdn.com/ie/archive/2004/10/26/247912.aspx .

I'll get to it eventually myself as well, not sure when though.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:24>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------------+------------------------

Comment (by marcob):

mrts I followed your link and I discovered the "glich". Thank you!

I've data in my database that contain a ' so this line in the popup window
link raises a javascript error:
{{{
onclick="opener.dismissRelatedLookupPopup(window, 'A014', 'Servizi
connessi all&#39;Agricoltura'); return false;"
}}}

The problem is that &#39;

With IE it's enough to open the popup window to get the error, with
Firefox you need to click that line. For this I never got it with Firefox,
at least once IE helped me :-).

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:25>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------------+------------------------

Comment (by mrts):

Fixed with {{{.replace("'", "`")}}} as was originally intended (but
incorrectly implemented). I wonder if this is causes problems for Romanic
languages that use {{{`}}} for accents -- Marco, is it OK for Italian?
I.e. you would see ''Servizi connessi all{{{`}}}Agricoltura'' instead of
''Servizi connessi all'Agricoltura'' in the object link. Also, I wonder if
English speakers would be annoyed by seeing ''Aesop{{{`}}}s fables''
instead of ''Aesop's fables''.

Other fixes:
* more than 7 words are now truncated instead of the somewhat arbitrary
14 in the original patch. Even retaining 7 may be a few too many,
* proper XSS protection with `nameElem.innerHTML = ...
html_escape(chosenName) ...;`, also get rid of the ugliness mentioned a
few comments above
* add `?_popup=1` to the related object popup and dismiss on save
* consistent label usage.

Will upload the patch tomorrow.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:26>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------------+------------------------

Comment (by marcob):

Replying to [comment:26 mrts]:

> Fixed with {{{.replace("'", "`")}}} as was originally intended (but
incorrectly implemented). I wonder if this is causes problems for Romanic
languages that use {{{`}}} for accents -- Marco, is it OK for Italian?
I.e. you would see ''Servizi connessi all{{{`}}}Agricoltura'' instead of
''Servizi connessi all'Agricoltura'' in the object link. Also, I wonder if
English speakers would be annoyed by seeing ''Aesop{{{`}}}s fables''
instead of ''Aesop's fables''.

Unfortunately this isn't correct in italian. We use ' (apostrophe) between
truncated words ("all'agricoltura") and we sometimes use {{{`}}} (back-
prime? back-apex? how is it called?) with accented word ("menu`" instead
of "menù").

Would it be possible to fix it with {{{.replace("'", r"\'")}}} ?

I tried this manually and the error vanished:

{{{
onclick="opener.dismissRelatedLookupPopup(window, 'A014', 'Servizi

connessi all\'Agricoltura'); return false;"

}}}


> Will upload the patch tomorrow.

I'll surely try it. Thanks.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:27>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------------+------------------------

Comment (by mrts):

Marco, please review and test when you have time (checking with that same
failing case in IE is much appreciated). Using {{{`}}} was both a hack and
a bad idea, thanks for rectifying this (my only excuse, albeit a feeble
one, is that it doesn't matter in my language, {{{`}}} and {{{'}}} are
treated more or less equivalently).

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:28>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1

Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 1 |
---------------------------------------------------+------------------------
Changes (by mrts):

* needs_better_patch: 0 => 1

* needs_tests: 0 => 1

Comment:

Although the patch is quite usable, it needs
* more view tests, specifically for the template tag and
`options.py/ModelAdmin.response_change` when the next item is implemented
* when the object's label changes via the popup, it makes sense to update
it with !JavaScript in the DOM as well (similarly to
`dismissAddAnotherPopup`), there are TODOs in the patch for the places
that need updating.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:29>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 1 |
---------------------------------------------------+------------------------

Comment (by marcob):

Tested and it works! Both in FF than IE. Thanks!

I was to write about update the label if changed when I saw you already
write it in the "it needs" part of you comment.
Just to know, are you willing to do it or you left it as an exercise for
the reader? :)

Another minor glitch is that I have a table with a FK on a table of
hierarchical codes that has a "self" FK. When I open the first popup
window and I click on the inside label for the "self" FK I don't get
another popup but it opens in the same window. I'm not sure if it's better
in this way or to open a new window but to obtain this behaviour it's
enough to name the window with the id of the object.

Btw this is nitpicking. You did a wonderful job.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:30>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 1 |
---------------------------------------------------+------------------------
Comment (by marcob):

mtrs, after looking at your patch I remembered this comment of mine:
http://code.djangoproject.com/ticket/6903#comment:30

Fanny, isn't it? I would like to say "great minds think alike" but perhaps
is more proper "great asses fart alike" :-)

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:31>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 1 |
---------------------------------------------------+------------------------
Comment (by marcob):

fanny = funny :) (Italian common error)

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:32>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 1 |
---------------------------------------------------+------------------------

Comment (by mrts):

:)

Your nitpicking is most welcome, thanks!

I plan continue taking care of the patch and I'll think about the popup
cascading case as well as implement the things mentioned in the TODO
comments. However, it is unclear when I have time for it. But I'd say the
patch is usable as-is for those who need it.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:33>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 1 |
---------------------------------------------------+------------------------

Comment (by marcob):

Mrts, I think that we need also to update the label during an add-another:
{{{
diff -r be3cfbfcb85b
lib/django/contrib/admin/media/js/admin/RelatedObjectLookups.js
--- a/lib/django/contrib/admin/media/js/admin/RelatedObjectLookups.js
Sat Dec 05 03:10:26 2009 +0100
+++ b/lib/django/contrib/admin/media/js/admin/RelatedObjectLookups.js
Sun Dec 06 06:00:58 2009 +0100
@@ -100,6 +100,13 @@
SelectBox.add_to_cache(toId, o);
SelectBox.redisplay(toId);
}
+ var nameElem = document.getElementById("view_lookup_" + name);
+ if (nameElem) {
+ var chosedIdHref = String(win.location.href).split(/\/add\//)[0] +
'/' + newId + '/';
+ nameElem.innerHTML = '<a href="' + chosedIdHref + '" ' +
+ 'onclick="return showRelatedObjectPopup(this);">' +
+ html_escape(newRepr) + '</a>';
+ }
win.close();
}
}}}

Do you agree?

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:34>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 1 |
---------------------------------------------------+------------------------

Comment (by mrts):

Marco, I've been ill and away, sorry for that. Will get back to this soon.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:35>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 1 |
---------------------------------------------------+------------------------

Comment (by marcob):

Nothing is due and you did most of the job on this patch :-)

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:36>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 1 |
---------------------------------------------------+------------------------

Comment (by mrts):

Marco, good point, but here come some of my notes:

1. the added block should be within the `} else if (elem.nodeName ==
'INPUT') {` branch,
1. as `win.location.href` already is a string, there is no need to cast
it,
1. we should not `split()` as theoretically it's not impossible that the
path already contains an add, e.g. a contrived but not illegal
'/add/admin/add/add/` (admin is mounted under `/add/`, there's an app
labeled `add`), so I propose the following instead: `var chosenIdHref =
win.location.href.replace(/\/add\/[^\/]*$/, '/' + newId + '/');` (that
gets rid of the redundant `?_popup=1` as well),
1. it makes sense to store the escaped copy of newRepr in the beginning
and not escape it again.

Thanks! I've updated the patch accordingly, re-merged master and will
upload the resulting patch shortly.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:37>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1

Needs_better_patch: 0 |
---------------------------------------------------+------------------------
Changes (by mrts):

* needs_better_patch: 1 => 0

Comment:

The linked label should properly update now both when objects are added
and changed via the popup, so all the required functionality should be
there.

Marco and others who are interested in this, please test and report any
problems you notice (e.g. I haven't tried in IE).

I'll leave it into "more tests needed" state as of now -- the
`options.py:response_change():elif request.POST.has_key("_popup")` path
needs testing as do quirky PKs and other corner cases.

> Another minor glitch is that I have a table with a FK on a table of
hierarchical codes that has a "self" FK. When I open the first popup
window and I click on the inside label for
> the "self" FK I don't get another popup but it opens in the same window.
I'm not sure if it's better in this way or to open a new window but to
obtain this behaviour it's enough
> to name the window with the id of the object.

I'd say handling this is not worth the effort. But you are most welcome to
tinker with the case.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:38>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 0 |
---------------------------------------------------+------------------------

Comment (by mrts):

I've added selection clearing via javascript with the red x button. The ID
input field (and the confusing numeric ID) can be entirely hidden from
end-users with CSS `display: none` now with no loss of functionality.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:39>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1

Needs_better_patch: 1 |
---------------------------------------------------+------------------------
Changes (by mrts):

* needs_better_patch: 0 => 1

Comment:

Note that I haven't even thought about !ManyToManyFields, but the patch
has to handle them as well eventually. Will deal with it later, left a
failing test as a remainder.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:40>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 1 |
---------------------------------------------------+------------------------

Comment (by mrts):

I've been asked for a 1.1 version by email. See
http://github.com/mrts/django/tree/ticket7028-1.1.X .

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:41>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 1 |
---------------------------------------------------+------------------------
Comment (by mrts):

While working at this, it would make sense also to fix #5704, #7955 and
perhaps #11683, #11684 as well.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:42>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 1 |
---------------------------------------------------+------------------------
Comment (by mrts):

A holistic approach would also take care of #11700.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:43>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 1 |
---------------------------------------------------+------------------------

Comment (by marcob):

mrts, I do agree, but who is in charge to close and link those tickets to
this one?

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:44>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 1 |
---------------------------------------------------+------------------------

Comment (by mrts):

A core committer should do this I think. I will presently leave the patch
as-is unless instructed otherwise.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:45>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN

Resolution: | Keywords: raw-id-fields nfa-someday design_ux

Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 1 |
---------------------------------------------------+------------------------

Changes (by russellm):

* keywords: raw-id-fields nfa-someday => raw-id-fields nfa-someday
design_ux

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:46>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday design_ux
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 1 |
---------------------------------------------------+------------------------

Comment (by mrts):

I created a 3-minute screencast of the new functionality, to make high-
level reviewing this easier: http://www.vimeo.com/9322083

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:47>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday design_ux
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 1 |
---------------------------------------------------+------------------------

Comment (by marcob):

Mart, one word: GREAT!

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:48>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone:

Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday design_ux
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 1 |
---------------------------------------------------+------------------------

Changes (by ubernostrum):

* milestone: 1.2 =>

Comment:

1.2 is feature-frozen, moving this feature request off the milestone.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:49>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone:
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday design_ux
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 1 |
---------------------------------------------------+------------------------

Comment (by dschruth):

I hate to do this, but the functionality is merely only close to what I
need but not quite there yet. I happen to like (or tolerate rather) the
dropdown box for the ForeignKeyField (probably because I make extensive
use of def __unicode__ to make my listed names more
verbose/descriptive)... and have no need for the pop-up functionality.

Consequently, all I need would be just the linking functionality to the
selected Foreign Object. I envision something similar to the way in which
the ImageField and the FileField link to a URL of the file/image: with the
link *just* *above* the "select" HTML form element.

I've reopened this ticket's 'duplicate' here
http://code.djangoproject.com/ticket/12085 for the first issue.

Additionally: It seems like there could be a similarly nice way to link to
the foreign key's class' change_list too. Perhaps we could just turn the
field's label into a simple link to the class' list table? This second
idea should probably be opened up as a separate ticket though...

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:50>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone:
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday design_ux
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 1 |
---------------------------------------------------+------------------------

Comment (by russellm):

#13221 was submitted with an alternate patch.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:51>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone:
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday design_ux
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 1 |
---------------------------------------------------+------------------------

Comment (by carwyn):

What's blocking progress on one or other of these patches? There seems to
be a lot of good work here on a worthy improvement to the admin interface.

In absence of accepted patches does it make sense to re-craft these
patches as widgets that could be used with ModelAdmin.formfield_overrides?

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:52>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone:
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday design_ux
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 1 |
---------------------------------------------------+------------------------

Comment (by mrts):

My patch is complete, but doesn't touch ManyToManyFields. I can not drive
this further though for various reasons, so someone else should step up
and continue the work.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:53>

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone:
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday design_ux
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 1 |
---------------------------------------------------+------------------------
Comment (by mrts):

`getAdminMediaPrefix()` is unneccessary in 1.2. See #11967 and [13002].
Use `if (window.__admin_media_prefix__ != undefined) { ...` etc.

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:54>

Django <http://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To post to this group, send email to django-...@googlegroups.com.
To unsubscribe from this group, send email to django-update...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/django-updates?hl=en.

[Django]

#7028: Better raw_id_fields feedback in newform-admins branch
---------------------------------------------------+------------------------
Reporter: Marcob <marc...@gmail.com> | Owner: nobody
Status: new | Milestone:
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords: raw-id-fields nfa-someday design_ux
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 1
Needs_better_patch: 1 |
---------------------------------------------------+------------------------

Comment (by EmilStenstrom):

While you wait for this ticket to be resolved, I've added a snippet that
shows the values for both ForeignKeys and ManyToManyFields, and links each
value to its change page in the admin. If this is something that can be
used in this patch, feed free to pick whatever you want from it:
http://djangosnippets.org/snippets/2217/

--
Ticket URL: <http://code.djangoproject.com/ticket/7028#comment:55>