CKAN: Ticket #1004: Group creation instructions missing

priority, milestone changed; keywords set

rgrp — Mon, 07 Mar 2011 17:16:49 GMT

keywords bitesize added
priority changed from awaiting triage to minor
milestone changed from ckan-v1.4-sprint-2 to ckan-v1.4

milestone changed; repo, state, theme set

dread — Mon, 16 May 2011 15:21:35 GMT

repo set to ckan
state set to draft
theme set to none
milestone changed from ckan-v1.4 to ckan-v1.5

milestone changed

thejimmyg — Wed, 20 Jul 2011 14:52:40 GMT

milestone changed from ckan-v1.5 to ckan-backlog

At the moment it says "Create a new group" and clicking it takes you to the login page. We want it to do the same thing but if you aren't logged in the label should say "Log in to create a group".

status changed; resolution set

thejimmyg — Tue, 06 Dec 2011 11:36:15 GMT

status changed from new to closed
resolution set to fixed

The yellow box on the right is now back and you don't get taken to the login page. We'll write more detailed instructions once the group refactor is done.

status changed; resolution deleted

dread — Tue, 06 Dec 2011 11:54:49 GMT

status changed from closed to reopened
resolution fixed deleted

There is a description of groups, but that is not the subject of this ticket. In fact if you're not logged in you do get taken to the login page (which is quite correct). I agree with your earlier comment that the text of the link needs changing.

Sorry to be reopening this, but I think the previous comment misses the point. Doing this ticket would be a valuable quick fix. Putting on backlog.

status changed; resolution set

thejimmyg — Tue, 06 Dec 2011 12:29:57 GMT

status changed from reopened to closed
resolution set to duplicate

I don't understand. We just had a team meeting about this, all discussed it and agreed to close it. Yes, you get taken to the login page, but that is the correct behaviour.

The problem in the past was that the link was in the yellow box and there was no explanation as to why you had to login. This time you click from the header bar and there is a clear message saying "Unauthorized to create a group" - exactly as a user would expect. Even if the exact text of the ticket description isn't fully implemented in the current release, the UX isn't broken anymore. Yes, it might be even nicer to have a message warning them in advanced but these improvements will be taken forward in the UX work - maybe there is an even better solution than a message?

Since you are unhappy about closing it I'm marking it as "Duplicate" of #1521. As agreed earlier with the entire team, we'll take this forward as part of the groups refactor.

See #1521 for more information.

status changed; resolution deleted

dread — Tue, 06 Dec 2011 13:26:34 GMT

status changed from closed to reopened
resolution duplicate deleted

My suggested fix:

diff --git a/ckan/templates/group/index.html b/ckan/templates/group/index.html
index 8502df9..064e066 100644
--- a/ckan/templates/group/index.html
+++ b/ckan/templates/group/index.html
@@ -9,7 +9,15 @@
   <py:match path="primarysidebar">
     <li class="widget-container boxed widget_text">
       <h3>What Are Groups?</h3>
-      <span i18n:msg="">Whilst tags are great at collecting datasets together,
+      <p><span i18n:msg="">Whilst tags are great at collecting datasets togethe
+      <p>
+      <span i18n:msg="" class="ckan-logged-in" style="display: none;">
+         <a href="${h.url_for(controller='group',action='new', id=None)}">Creat
+      </span>
+      <span i18n:msg="" class="ckan-logged-out">
+         To create a new group, please first <a href="${h.url_for(controller='u
+      </span>
+      </p>
     </li>
   </py:match>
diff --git a/ckan/templates/group/layout.html b/ckan/templates/group/layout.html
index 64153aa..a3f732b 100644
--- a/ckan/templates/group/layout.html
+++ b/ckan/templates/group/layout.html
@@ -25,9 +25,12 @@
       <li py:attrs="{'class':'current-tab'} if c.action=='index' else {}">
         ${h.subnav_link(c, h.icon('group') + _('List Groups'), controller='grou
       </li>
-      <li py:attrs="{'class':'current-tab'} if c.action=='new' else {}">
+      <li py:attrs="{'class':'current-tab'} if c.action=='new' else {}" class="
         ${h.subnav_link(c, h.icon('group_add') + _('Add a Group'), controller='
       </li>
+      <li py:attrs="{'class':'current-tab'} if c.action=='new' else {}" class="
+        ${h.subnav_link(c, h.icon('group_add') + _('Log-in to add a Group'), co
+      </li>
     </ul>
   </py:match>

BUT this fix doesn't completely work. When you log-in and create a group, at this point the nav bar changes from the (correct) "Add a group" to (incorrect) "Log-in to add a group".

status changed; owner set

dread — Tue, 06 Dec 2011 13:26:45 GMT

owner set to zephod
status changed from reopened to assigned

dread — Tue, 06 Dec 2011 16:02:32 GMT

Here is the unclipped version:

      <p>
      <span i18n:msg="" class="ckan-logged-in" style="display: none;">
	  <a href="${h.url_for(controller='group',action='new', id=None)}">Create a new group</a>
      </span>
      <span i18n:msg="" class="ckan-logged-out">
	  To create a new group, please first <a href="${h.url_for(controller='user',action='login', id=None)}">log-in</a>.
      </span>
      </p>

      <li py:attrs="{'class':'current-tab'} if c.action=='new' else {}" class="ckan-logged-in" style="display: none;">
        ${h.subnav_link(c, h.icon('group_add') + _('Add a Group'), controller='group', action='new')}
      </li>
      <li py:attrs="{'class':'current-tab'} if c.action=='new' else {}" class="ckan-logged-out">
        ${h.subnav_link(c, h.icon('group_add') + _('Log-in to add a Group'), controller='group', action='new')}
      </li>

status changed; resolution set

zephod — Tue, 06 Dec 2011 17:53:36 GMT

status changed from assigned to closed
resolution set to fixed

Dread, that fix wouldn't work because the class attribute was being defined in Genshi and again in HTML, with slighly undefined behaviour (in this case Genshi overwrites HTML). I have performed a very very simple fix which modifies the navbar text when you're not logged in. https://github.com/okfn/ckan/commit/a89a48731ba548170045a60ac2930e0019c299c7

I don't think the link should be restored in the sidebar, it was explicitly removed as part of a site-wide sweep to make the sidebar a passive, helpful element rather than an active element with action links.

Site looks great to me, I'm closing this ticket.

dread — Tue, 06 Dec 2011 18:18:05 GMT

Great stuff zeph! Diff looks really good and cheers for the explanation.

On master, cset:a89a48731ba548170045a60ac2930e0019c299c7 and I've cherry picked this for release 1.5.1. too.