Ticket #1004 (closed defect: fixed)

Opened 3 years ago

Last modified 2 years ago

Group creation instructions missing

Reported by: dread Owned by: zephod
Priority: minor Milestone: ckan-backlog
Component: ckan Keywords: bitesize
Cc: Repository: ckan
Theme: none

Description

Need instructions on the group page to tell people they need to login to create a group. Someone must have deleted this.

Change History

comment:1 Changed 3 years ago by rgrp

  • Keywords bitesize added
  • Priority changed from awaiting triage to minor
  • Milestone changed from ckan-v1.4-sprint-2 to ckan-v1.4

comment:2 Changed 3 years ago by dread

  • Repository set to ckan
  • state set to draft
  • Theme set to none
  • Milestone changed from ckan-v1.4 to ckan-v1.5

comment:3 Changed 3 years ago by thejimmyg

  • 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".

comment:4 Changed 2 years ago by thejimmyg

  • 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.

comment:5 Changed 2 years ago by dread

  • 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.

comment:6 Changed 2 years ago by thejimmyg

  • 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.

comment:7 Changed 2 years ago by dread

  • 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".

comment:8 Changed 2 years ago by dread

  • Owner set to zephod
  • Status changed from reopened to assigned

comment:9 Changed 2 years ago by dread

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>

comment:10 Changed 2 years ago by zephod

  • 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.

comment:11 Changed 2 years ago by dread

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.

Note: See TracTickets for help on using tickets.