chiark / gitweb /
Translate preferences page to the new order.
authorRichard Kettlewell <rjk@greenend.org.uk>
Sun, 18 May 2008 15:18:49 +0000 (16:18 +0100)
committerRichard Kettlewell <rjk@greenend.org.uk>
Sun, 18 May 2008 15:18:49 +0000 (16:18 +0100)
Also, this change fixes much of defect 18 ("cookie expiry causes user
to be silently logged out and not subsequently redirected to login
page").  This is implemented by giving the actions table a list of
rights suitable for the action at hand and redirecting to the login
page on failure.  Even better would be to redirect back to the
problematic page on success.

Unusually for this branch there is a server change here, implementing
the long-standing TODO that the server should remove preferences if
you set them to their default value.  This removes this logic from the
web interface.

It remains the case that the web interface has to work out some of the
defaults.  However, that's left for another day.

lib/trackdb.c
server/actions.c
server/dcgi.c
templates/choose.tmpl
templates/macros.tmpl
templates/new.tmpl
templates/options.labels
templates/prefs.tmpl
templates/recent.tmpl

index 54016ba..8e775d5 100644 (file)
@@ -1387,6 +1387,60 @@ void trackdb_stats_subprocess(ev_source *ev,
   ev_reader_new(ev, p[0], stats_read, stats_error, d, "disorder-stats reader");
 }
 
+/** @brief Parse a track name part preference
+ * @param name Preference name
+ * @param partp Where to store part name
+ * @param contextp Where to store context name
+ * @return 0 on success, non-0 if parse fails
+ */
+static int trackdb__parse_namepref(const char *name,
+                                   char **partp,
+                                   char **contextp) {
+  char *c;
+  static const char prefix[] = "trackname_";
+  
+  if(strncmp(name, prefix, strlen(prefix)))
+    return -1;                          /* not trackname_* at all */
+  name += strlen(prefix);
+  /* There had better be a _ between context and part */
+  c = strchr(name, '_');
+  if(!c)
+    return -1;
+  /* Context is first in the pref name even though most APIs have the part
+   * first.  Confusing; sorry. */
+  *contextp = xstrndup(name, c - name);
+  ++c;
+  /* There had better NOT be a second _ */
+  if(strchr(c, '_'))
+    return -1;
+  *partp = xstrdup(c);
+  return 0;
+}
+
+/** @brief Compute the default value for a track preference
+ * @param track Track name
+ * @param name Preference name
+ * @return Default value or 0 if none/not known
+ */
+static const char *trackdb__default(const char *track, const char *name) {
+  char *context, *part;
+  
+  if(!trackdb__parse_namepref(name, &part, &context)) {
+    /* We can work out the default for a trackname_ pref */
+    return trackname_part(track, context, part);
+  } else if(!strcmp(name, "weight")) {
+    /* We know the default weight */
+    return "90000";
+  } else if(!strcmp(name, "pick_at_random")) {
+    /* By default everything is eligible for picking at random */
+    return "1";
+  } else if(!strcmp(name, "tags")) {
+    /* By default everything no track has any tags */
+    return "";
+  }
+  return 0;
+}
+
 /* set a pref (remove if value=0) */
 int trackdb_set(const char *track,
                 const char *name,
@@ -1395,9 +1449,15 @@ int trackdb_set(const char *track,
   DB_TXN *tid;
   int err, cmp;
   char *oldalias, *newalias, **oldtags = 0, **newtags;
+  const char *def;
 
+  /* If the value matches the default then unset instead, to keep the database
+   * tidy.  Older versions did not have this feature so your database may yet
+   * have some default values stored in it. */
   if(value) {
-    /* TODO: if value matches default then set value=0 */
+    def = trackdb__default(track, name);
+    if(def && !strcmp(value, def))
+      value = 0;
   }
 
   for(;;) {
index 4754a8c..c53575e 100644 (file)
@@ -190,7 +190,7 @@ static void act_play(void) {
   struct dcgi_entry *e;
   
   if(dcgi_client) {
-    if((track = cgi_get("file"))) {
+    if((track = cgi_get("track"))) {
       disorder_play(dcgi_client, track);
     } else if((dir = cgi_get("dir"))) {
       if(disorder_files(dcgi_client, dir, 0, &tracks, &ntracks))
@@ -481,31 +481,98 @@ static void act_reminder(void) {
   dcgi_expand("login", 1);
 }
 
+/** @brief Get the numbered version of an argument
+ * @param argname Base argument name
+ * @param numfile File number
+ * @return cgi_get(NUMFILE_ARGNAME)
+ */
+static const char *numbered_arg(const char *argname, int numfile) {
+  char *fullname;
+
+  byte_xasprintf(&fullname, "%d_%s", numfile, argname);
+  return cgi_get(fullname);
+}
+
+/** @brief Set preferences for file @p numfile
+ * @return 0 on success, -1 if there is no such track number
+ *
+ * The old @b nfiles parameter has been abolished, we just keep look for more
+ * files until we run out.
+ */
+static int process_prefs(int numfile) {
+  const char *file, *name, *value, *part, *parts, *context;
+  char **partslist;
+
+  if(!(file = numbered_arg("track", numfile)))
+    return -1;
+  if(!(parts = cgi_get("parts")))
+    parts = "artist album title";
+  if(!(context = cgi_get("context")))
+    context = "display";
+  partslist = split(parts, 0, 0, 0, 0);
+  while((part = *partslist++)) {
+    if(!(value = numbered_arg(part, numfile)))
+      continue;
+    byte_xasprintf((char **)&name, "trackname_%s_%s", context, part);
+    disorder_set(dcgi_client, file, name, value);
+  }
+  if((value = numbered_arg("random", numfile)))
+    disorder_unset(dcgi_client, file, "pick_at_random");
+  else
+    disorder_set(dcgi_client, file, "pick_at_random", "0");
+  if((value = numbered_arg("tags", numfile))) {
+    if(!*value)
+      disorder_unset(dcgi_client, file, "tags");
+    else
+      disorder_set(dcgi_client, file, "tags", value);
+  }
+  if((value = numbered_arg("weight", numfile))) {
+    if(!*value)
+      disorder_unset(dcgi_client, file, "weight");
+    else
+      disorder_set(dcgi_client, file, "weight", value);
+  }
+  return 0;
+}
+
+static void act_set(void) {
+  int numfile;
+
+  if(dcgi_client) {
+    for(numfile = 0; !process_prefs(numfile); ++numfile)
+      ;
+  }
+  redirect(0);
+}
+
 /** @brief Table of actions */
 static const struct action {
   /** @brief Action name */
   const char *name;
   /** @brief Action handler */
   void (*handler)(void);
+  /** @brief Union of suitable rights */
+  rights_type rights;
 } actions[] = {
-  { "confirm", act_confirm },
-  { "disable", act_disable },
-  { "edituser", act_edituser },
-  { "enable", act_enable },
-  { "login", act_login },
-  { "logout", act_logout },
-  { "manage", act_playing },
-  { "move", act_move },
-  { "pause", act_pause },
-  { "play", act_play },
-  { "playing", act_playing },
-  { "randomdisable", act_random_disable },
-  { "randomenable", act_random_enable },
-  { "register", act_register },
-  { "reminder", act_reminder },
-  { "remove", act_remove },
-  { "resume", act_resume },
-  { "volume", act_volume },
+  { "confirm", act_confirm, 0 },
+  { "disable", act_disable, RIGHT_GLOBAL_PREFS },
+  { "edituser", act_edituser, 0 },
+  { "enable", act_enable, RIGHT_GLOBAL_PREFS },
+  { "login", act_login, 0 },
+  { "logout", act_logout, 0 },
+  { "manage", act_playing, 0 },
+  { "move", act_move, RIGHT_MOVE__MASK },
+  { "pause", act_pause, RIGHT_PAUSE },
+  { "play", act_play, RIGHT_PLAY },
+  { "playing", act_playing, 0 },
+  { "randomdisable", act_random_disable, RIGHT_GLOBAL_PREFS },
+  { "randomenable", act_random_enable, RIGHT_GLOBAL_PREFS },
+  { "register", act_register, 0 },
+  { "reminder", act_reminder, 0 },
+  { "remove", act_remove, RIGHT_MOVE__MASK|RIGHT_SCRATCH__MASK },
+  { "resume", act_resume, RIGHT_PAUSE },
+  { "set", act_set, RIGHT_PREFS },
+  { "volume", act_volume, RIGHT_VOLUME },
 };
 
 /** @brief Check that an action name is valid
@@ -579,10 +646,20 @@ void dcgi_action(const char *action) {
     /* Make sure 'action' is always set */
     cgi_set("action", action);
   }
-  if((n = TABLE_FIND(actions, struct action, name, action)) >= 0)
-    /* Its a known action */
+  if((n = TABLE_FIND(actions, struct action, name, action)) >= 0) {
+    if(actions[n].rights) {
+      /* Some right or other is required */
+      dcgi_lookup(DCGI_RIGHTS);
+      if(!(actions[n].rights & dcgi_rights)) {
+        /* Failed operations jump you to the login screen with an error
+         * message */
+        login_error("noright");
+        return;
+      }
+    }
+    /* It's a known action */
     actions[n].handler();
-  else {
+  else {
     /* Just expand the template */
     dcgi_expand(action, 1/*header*/);
   }
index 9a510c8..bf6fbe5 100644 (file)
@@ -80,94 +80,8 @@ static void expand_template(dcgi_state *ds, cgi_sink *output,
   expand(output, action, ds);
 }
 
-/* actions ********************************************************************/
-
-static void act_prefs_errors(const char *msg,
-                            void attribute((unused)) *u) {
-  fatal(0, "error splitting parts list: %s", msg);
-}
-
-static const char *numbered_arg(const char *argname, int numfile) {
-  char *fullname;
-
-  byte_xasprintf(&fullname, "%d_%s", numfile, argname);
-  return cgi_get(fullname);
-}
-
-static void process_prefs(dcgi_state *ds, int numfile) {
-  const char *file, *name, *value, *part, *parts, *current, *context;
-  char **partslist;
-
-  if(!(file = numbered_arg("file", numfile)))
-    /* The first file doesn't need numbering. */
-    if(numfile > 0 || !(file = cgi_get("file")))
-      return;
-  if((parts = numbered_arg("parts", numfile))
-     || (parts = cgi_get("parts"))) {
-    /* Default context is display.  Other contexts not actually tested. */
-    if(!(context = numbered_arg("context", numfile))) context = "display";
-    partslist = split(parts, 0, 0, act_prefs_errors, 0);
-    while((part = *partslist++)) {
-      if(!(value = numbered_arg(part, numfile)))
-       continue;
-      /* If it's already right (whether regexps or db) don't change anything,
-       * so we don't fill the database up with rubbish. */
-      if(disorder_part(ds->g->client, (char **)&current,
-                      file, context, part))
-       fatal(0, "disorder_part() failed");
-      if(!strcmp(current, value))
-       continue;
-      byte_xasprintf((char **)&name, "trackname_%s_%s", context, part);
-      disorder_set(ds->g->client, file, name, value);
-    }
-    if((value = numbered_arg("random", numfile)))
-      disorder_unset(ds->g->client, file, "pick_at_random");
-    else
-      disorder_set(ds->g->client, file, "pick_at_random", "0");
-    if((value = numbered_arg("tags", numfile))) {
-      if(!*value)
-       disorder_unset(ds->g->client, file, "tags");
-      else
-       disorder_set(ds->g->client, file, "tags", value);
-    }
-    if((value = numbered_arg("weight", numfile))) {
-      if(!*value || !strcmp(value, "90000"))
-       disorder_unset(ds->g->client, file, "weight");
-      else
-       disorder_set(ds->g->client, file, "weight", value);
-    }
-  } else if((name = cgi_get("name"))) {
-    /* Raw preferences.  Not well supported in the templates at the moment. */
-    value = cgi_get("value");
-    if(value)
-      disorder_set(ds->g->client, file, name, value);
-    else
-      disorder_unset(ds->g->client, file, name);
-  }
-}
-
-static void act_prefs(cgi_sink *output, dcgi_state *ds) {
-  const char *files;
-  int nfiles, numfile;
-
-  if((files = cgi_get("files"))) nfiles = atoi(files);
-  else nfiles = 1;
-  for(numfile = 0; numfile < nfiles; ++numfile)
-    process_prefs(ds, numfile);
-  cgi_header(output->sink, "Content-Type", "text/html");
-  header_cookie(output->sink);
-  cgi_body(output->sink);
-  expand(output, "prefs", ds);
-}
 /* expansions *****************************************************************/
 
-static void exp_label(int attribute((unused)) nargs,
-                     char **args,
-                     cgi_sink *output,
-                     void attribute((unused)) *u) {
-  cgi_output(output, "%s", cgi_label(args[0]));
-}
-
 struct trackinfo_state {
   dcgi_state *ds;
   const struct queue_entry *q;
index fcba496..4300852 100644 (file)
@@ -125,7 +125,7 @@ USA
      @right{prefs}{
      <td class=imgbutton>
       <a class=imgbutton
-         href="@url?action=prefs&#38;0_file=@urlquote{@track}">
+         href="@url?action=prefs&#38;track=@urlquote{@track}">
        <img class=button src="@image{edit}"
             title="@label{choose.prefsverbose}"
             alt="@label{choose.prefs}">
@@ -181,14 +181,14 @@ USA
 @define{sometracks}{template}{@template}@#
        @right{prefs}{
         <a class=imgprefs
-           href="@url?action=prefs&#38;0_file=@urlquote{@resolve{@track}}">
+           href="@url?action=prefs&#38;track=@urlquote{@resolve{@track}}">
          <img class=button
               src="@image{edit}"
               title="@label{choose.prefsverbose}"
               alt="@label{choose.prefs}">
         </a>
        }@#
-       <a href="@url?action=play&#38;file=@urlquote{@track}&#38;back=@urlquote{@thisurl}"
+       <a href="@url?action=play&#38;track=@urlquote{@track}&#38;back=@urlquote{@thisurl}"
           title="@label{choose.play}">
         @display
        </a>
index 495c29a..3e2bc0b 100644 (file)
@@ -69,8 +69,8 @@ and then redefines macros as desired.
 @define {menuitem} {current name available}  
         {@if{@available}
             {   <a @if{@eq{@current}{@name}}
-                     {class=activemenu}
-                     {class=inactivemenu}
+                      {class=activemenu}
+                      {class=inactivemenu}
 @if{@eq{name}{playing}}
    {      href="@url"}
    {      href="@url?action=@name"}
@@ -136,7 +136,7 @@ and then redefines macros as desired.
 @#  @what is the section
 @#  @track is the track name
 @define {mtitleplay} {what track}
-        {<a title="@part{@track}{title}" href="@url?action=play&#38;file=@urlquote{@track}&#38;back=@urlquote{@thisurl}">@part{@track}{title}{short}</a>}
+        {<a title="@part{@track}{title}" href="@url?action=play&#38;track=@urlquote{@track}&#38;back=@urlquote{@thisurl}">@part{@track}{title}{short}</a>}
 
 @# Expand to the remove/scratch entry for @id
 @#  @what is the section
@@ -174,6 +174,75 @@ and then redefines macros as desired.
                   title="@label{playing.@q{@dir}verbose}"
                   alt="@label{playing.@dir}">}}
 
+@# Size of input box for preferences forms
+@define{prefsize}{}{40}
+
+@# Expand to the weight of a track.  This macro knows the default weight,
+@# and does two lookups, which is rather inelegant.
+@#  @track is the track name.
+@define{weight}{track}{@if{@eq{@pref{@track}{weight}}{}} 
+                          {90000}
+                          {@pref{@track}{weight}}}
+
+@# Expand to preference form section for a track
+@#  @index is the track number
+@#  @track is the track name
+@define {mprefs} {index track}
+        {
+   <p class="prefs_head">Preferences for <span class="prefs_track">@quote{@resolve{@track}}</span>:</p>
+   <input type=hidden name="@index@__track" value="@quote{@resolve{@track}}">
+   <table class=prefs>
+    <tr class=headings>
+     <th class="prefs_name">@label{prefs.name}</th>
+     <th class="prefs_value">@label{prefs.value}</th>
+    </tr>
+    <tr class=even>
+     <td class="prefs_name">@label{heading.title}</td>
+     <td class="prefs_value">
+      <input size=@prefsize type=text name="@index@__title"
+             value="@part{@track}{title}{display}">
+     </td>
+    </tr>
+    <tr class=odd>
+     <td class="prefs_name">@label{heading.album}</td>
+     <td class="prefs_value">
+      <input size=@prefsize type=text name="@index@__album"
+             value="@part{@track}{album}{display}">
+     </td>
+    </tr>
+    <tr class=even>
+     <td class="prefs_name">@label{heading.artist}</td>
+     <td class="prefs_value">
+      <input size=@prefsize type=text name="@index@__artist"
+             value="@part{@track}{artist}{display}">
+     </td>
+    </tr>
+    <tr class=odd>
+     <td class="prefs_name">@label{prefs.tags}</td>
+     <td class="prefs_value">
+      <input size=@prefsize type=text name="@index@__tags"
+             value="@pref{@track}{tags}">
+     </td>
+    </tr>
+    <tr class=even>
+     <td class="prefs_name">@label{prefs.weight}</td>
+     <td class="prefs_value">
+      <input size=@prefsize type=text name="@index@__weight"
+             value="@weight{@track}">
+     </td>
+    </tr>
+    <tr class=odd>
+     <td class="prefs_name">@label{prefs.random}</td>
+     <td class="prefs_value">
+      <input type=checkbox value=true
+             name="@index@__random" 
+             @if{@ne{@pref{@track}{pick_at_random}}{0}}
+                {checked}>
+      </td>
+    </tr>
+   </table>
+}
+
 Local variables:
 mode:sgml
 sgml-always-quote-attributes:nil
index 6611f08..9de5ccd 100644 (file)
@@ -46,7 +46,7 @@ USA
      @right{prefs}{
      <td class=imgbutton>
       <a class=imgbutton
-         href="@url?action=prefs&#38;0_file=@urlquote{@track}">
+         href="@url?action=prefs&#38;track=@urlquote{@track}">
        <img class=button src="@image{edit}"
             title="@label{choose.prefsverbose}"
             alt="@label{choose.prefs}">
index a897b1f..c4aac36 100644 (file)
@@ -196,6 +196,7 @@ label       error.noconfirm         "Missing confirmation string."
 label  error.badconfirm        "Invalid confirmation string."
 label  error.badedit           "Cannot edit user details."
 label  error.reminderfailed    "Cannot send a reminder."
+label   error.noright           "Access denied."
 
 # Text appended to all error pages
 label  error.generic           ""
index db03220..74240da 100644 (file)
@@ -24,55 +24,26 @@ USA
  </head>
  <body>
 @stdmenu{prefs}
-   <h1>@label:prefs.title@</h1>
+  <h1>@label{prefs.title}</h1>
 
-   <form class=prefs action="@url@" method=POST
-         enctype="multipart/form-data" accept-charset=utf-8>
-    <input type=hidden name="files" value="@nfiles@">
-    <input type=hidden name=parts value="artist album title">
-   @files{
-   <p class="prefs_head">Preferences for <span class="prefs_track">@arg{@index@_file}@</span></p>
-    <input type=hidden name="@index@_file" value="@arg{@index@_file}@">
-    <table class=prefs>
-      <tr class="headings">
-       <th class="prefs_name">@label:prefs.name@</th>
-       <th class="prefs_value">@label:prefs.value@</th>
-      </tr>
-      <tr class=even>
-       <td class="prefs_name">@label:heading.title@</td>
-       <td class="prefs_value"><input size=40 type=text name="@index@_title" value="@part{@arg{@index@_file}display}{title}{display}"></td>
-      </tr>
-      <tr class=odd>
-       <td class="prefs_name">@label:heading.album@</td>
-       <td class="prefs_value"><input size=40 type=text name="@index@_album" value="@part{display}{album}{@arg{@index@_file}@}@"></td>
-      </tr>
-      <tr class=even>
-       <td class="prefs_name">@label:heading.artist@</td>
-       <td class="prefs_value"><input size=40 type=text name="@index@_artist" value="@part{display}{artist}{@arg{@index@_file}@}@"></td>
-      </tr>
-      <tr class=odd>
-       <td class="prefs_name">@label:prefs.tags@</td>
-       <td class="prefs_value"><input size=40 type=text name="@index@_tags" value="@pref{@arg{@index@_file}@}{tags}@"></td>
-      </tr>
-      <tr class=even>
-       <td class="prefs_name">@label:prefs.weight@</td>
-       <td class="prefs_value"><input size=40 type=text name="@index@_weight" value="@pref{@arg{@index@_file}@}{weight}@"></td>
-      </tr>
-      <tr class=odd>
-       <td class="prefs_name">@label:prefs.random@</td>
-       <td class="prefs_value"><input type=checkbox
-        name="@index@_random" value=true
-       @if{@ne{@pref{@arg{@index@_file}@}{pick_at_random}@}{0}@}{ checked}{}@></td>
-    </table>
-   }@
-    
-    <p>
-     <button type=submit name=submit>
-      @label:prefs.set@
-     </button>
-     <input name=action type=hidden value=prefs>
-    </p>
-   </form>
+  <form class=prefs method=POST
+        action="@url"
+        enctype="multipart/form-data"
+        accept-charset="utf-8">
+   <input type=hidden name=parts value="artist album title">
+   <input type=hidden name=context value="display">
+   <input name=action type=hidden value=set>
+   <input name=back type=hidden value="@quote{@thisurl}">
+   @if{@eq{@arg{dir}}{}}
+      {@mprefs{0}{@arg{track}}}
+      {@tracks{@arg{dir}}
+              {@mprefs{@index}{@track}}}
+   <p>
+    <button type=submit name=submit>
+     @label{prefs.set}
+    </button>
+   </p>
+  </form>
 
 @credits
  </body>
index 8f3fd9c..b43d01a 100644 (file)
@@ -49,7 +49,7 @@ USA
      @right{prefs}{
      <td class=imgbutton>
       <a class=imgbutton
-         href="@url?action=prefs&#38;0_file=@urlquote{@track}">
+         href="@url?action=prefs&#38;track=@urlquote{@track}">
        <img class=button src="@image{edit}"
             title="@label{choose.prefsverbose}"
             alt="@label{choose.prefs}">