Forum OpenACS Q&A: Breakthrough on bookmarks module

I now have a working bookmarks module, including the ability
to add and delete bookmarks, create folders, edit bookmarks (which
includes moving them from one folder to another), and open/close
folders.  The complete functionality as it exists in the original
ACS is not quite there, but it's getting close.

In order to accomplish this, I had to make several adjustments in the
original ACS code, including, but not limited to, the following:
    * bm_list_pkg becomes a simple table (not an Oracle package!)
    * some of the trigger mechanisms had to be moved from the data
      model to the edit-bookmark-2.tcl file, which is most likely OK,
      since the parent_id's of bookmarks probably only change when
      you use the "edit" part of the module, anyway
    * in place of the Oracle (proprietary) CONNECT BY construct,
      I decided to make judicious use of the "sort keys", making
      all recursive parent-child references unnecessary

I have all the fixes I've done so far ready and available, and I will
be posting them in the next day or two, as followup threads to this
posting.  Thus far, I've had to modify about 8 Tcl files, in addition
to the bookmarks.sql data model.

When I post the fixes, I'll try to be specific as to how the revised
data model works, and why I made the changes I did.

Stay tuned!

The revised data model, bookmarks.sql

-- bookmarks.sql

create sequence bm_url_id_seq;

create table bm_urls (
    url_id                  integer primary key,
    -- url title may be null in the case of bookmarks
    -- that are merely icons ie. AIM
    url_title               varchar(500),
    -- host url is separated from complete_url for counting purposes
    host_url                varchar(100) not null,
    complete_url            varchar(500) not null,
    -- meta tags that could be looked up regularly
    meta_keywords           varchar(4000),
    meta_description        varchar(4000),
    last_checked_date       datetime,
    -- the last time the site returned a "live" status
    last_live_date          datetime
);

-- this insertion is required for the change in create-folder-2.tcl
-- (and possibly elsewhere):
insert into bm_urls (url_id, host_url, complete_url)
  values (0, ' ', ' ');
-- (folders in bm_list are joined to a "dummy" entry in bm_urls, as above)

create sequence bm_bookmark_id_seq;
create table bm_list (
    bookmark_id             integer primary key,
    -- sort keys contains 3 characters per level of depth, from
    -- 0-9, then A-Z, a-z. You can get the depth as
    -- length(parent_sort_key) / 3.
    -- the full sort key for any bookmark is
    -- parent_sort_key || local_sort_key
    parent_sort_key         varchar(99), -- parent's sort key
    local_sort_key          char(3) not null,
    owner_id                integer not null references users(user_id),
    creation_date           datetime not null,
    modification_date       datetime,
    -- url_id may be null if the bookmark is a folder
    url_id                  integer references bm_urls,
    -- a person may rename any of his bookmarks so we keep a local title
    local_title                     varchar(500),
    private_p               char(1) default 'f' check (private_p in ('t','f')),
    -- needed in addition to private_p for the case where a public bookmark
    -- is under a hidden folder
    hidden_p                char(1) default 'f' check (hidden_p in ('t','f')),
    -- this is 't' if the bookmark is a folder
    folder_p                char(1) default 'f' check (folder_p in ('t','f')),
    -- null parent_id indicates this is a top level folder/bookmark
    parent_id               integer references bm_list(bookmark_id),
    -- refers to whether a folder is open or closed
    closed_p                char(1) default 't' check (closed_p in ('t','f')),
    -- whether the bookmark is within a closed folder and therefore not shown
    in_closed_p             char(1) default 'f' check (in_closed_p in ('t','f'))
);

CREATE function inc_char_for_sort_key (char)
  returns char(2)
AS '
DECLARE
   old_char char(2);
   carry_p  integer;
   old_code INTEGER;
   new_code INTEGER;
BEGIN
   old_char := $1;
   old_code := ascii(old_char);
   IF old_code = 57 THEN
      -- skip from 9 to A
      new_code := 65;
      carry_p := 0;
   ELSE IF old_code = 90 THEN
      -- skip from Z to a
      new_code := 97;
      carry_p := 0;
   ELSE IF old_code = 122 THEN
      -- wrap around
      new_code := 48;
      carry_p := 1;
   ELSE
      new_code := old_code + 1;
      carry_p := 0;
   END IF;
   END IF;
   END IF;
   --old_char := chr(new_code);
   old_char := ichar(new_code) || cast(carry_p as char(1));
   return old_char;
END;
--END inc_char_for_sort_key;
' language 'plpgsql';

CREATE FUNCTION new_sort_key (CHAR(3)) RETURNS CHAR(3)
AS '
DECLARE
   v_old_sort_key char(3);
   v_chr_1 char(2);
   v_chr_2 char(2);
   v_chr_3 char(2);
   v_carry INTEGER;
BEGIN
   v_old_sort_key := $1;
   IF v_old_sort_key IS null or v_old_sort_key = '''' THEN
      RETURN ''000'';
   END IF;

   v_chr_1 := substr(v_old_sort_key, 1, 1);
   v_chr_2 := substr(v_old_sort_key, 2, 1);
   v_chr_3 := substr(v_old_sort_key, 3, 1);

   v_chr_3 := inc_char_for_sort_key(v_chr_3);
   v_carry := cast(substr(v_chr_3, 2, 1) as integer);
   v_chr_3 := substr(v_chr_3, 1, 1);
   IF v_carry = 1 THEN
      v_chr_2 := inc_char_for_sort_key(v_chr_2);
      v_carry := cast(substr(v_chr_2, 2, 1) as integer);
      v_chr_2 := substr(v_chr_2, 1, 1);
      IF v_carry = 1 THEN
         v_chr_1 := inc_char_for_sort_key(v_chr_1);
         v_carry := cast(substr(v_chr_1, 2, 1) as integer);
         v_chr_1 := substr(v_chr_1, 1, 1);
      END IF;
   END IF;

   RETURN trim(v_chr_1) || trim(v_chr_2) || trim(v_chr_3);
END;
--END new_sort_key;
' language 'plpgsql';

create function trig_bm_list_sort_key_i_tr() returns opaque
as '
DECLARE
  v_last_sort_key bm_list.local_sort_key%TYPE;
  v_parent_sort_key bm_list.parent_sort_key%TYPE;
BEGIN
   IF NEW.parent_id IS NULL THEN
      SELECT max(local_sort_key) INTO v_last_sort_key
        FROM bm_list
        WHERE parent_id IS NULL;
      v_parent_sort_key := null;
   ELSE
      SELECT max(local_sort_key) INTO v_last_sort_key
        FROM bm_list
        WHERE parent_id = NEW.parent_id;
      SELECT parent_sort_key || cast(local_sort_key as varchar(3))
        INTO v_parent_sort_key
        FROM bm_list WHERE bookmark_id = NEW.parent_id;
   END IF;

   NEW.local_sort_key := new_sort_key(v_last_sort_key);
   NEW.parent_sort_key := v_parent_sort_key;
    RETURN NEW;
END;
' language 'plpgsql';

CREATE trigger bm_list_sort_key_i_tr
before INSERT ON bm_list
FOR each row
execute procedure trig_bm_list_sort_key_i_tr();

create table bm_list_pkg (v_updated_ids integer, v_num_entries integer);
insert into bm_list_pkg (v_num_entries) values (0);

create function trig_bm_list_sort_key_row_u_tr() returns opaque
as '
DECLARE
  old_parent_id integer;
  new_parent_id integer;
  f_bookmark_id integer;
  f_num_entries integer;
BEGIN
  old_parent_id := OLD.parent_id;
  new_parent_id := NEW.parent_id;
  IF old_parent_id is null THEN
    old_parent_id := 0;
  END IF;
  IF new_parent_id is null THEN
    new_parent_id := 0;
  END IF;
  IF old_parent_id != new_parent_id THEN
    f_bookmark_id := NEW.bookmark_id;
    select into f_num_entries max(v_num_entries)+1 from bm_list_pkg;
    insert into bm_list_pkg (v_updated_ids, v_num_entries)
      values (f_bookmark_id, f_num_entries);
  END IF;
  return NEW;
END;
' language 'plpgsql';

CREATE trigger bm_list_sort_key_row_u_tr
  before UPDATE ON bm_list
  FOR each row
execute procedure trig_bm_list_sort_key_row_u_tr();

-- the rest of the data model (functions and triggers) has moved,
-- for the time being, primarily, to edit-bookmark-2.tcl!

-- need two indices to support CONNECT BY

create index bm_list_idx1 on bm_list(bookmark_id, parent_id);
create index bm_list_idx2 on bm_list(parent_id, bookmark_id);

-- and the jury's still out on these two indexes, since our data
-- model has changed substantially (and does not use CONNECT BY)!

Changes in create-folder-2.tcl

Numbers 1 and 2 below reflect the change in the treatment of folders, which now have a dummy entry in bm_urls (as described in the data model).

1. Around line 39:

   (bookmark_id, owner_id, local_title, parent_id, creation_date, folder_p, closed_p, url_id)
   

   instead of

   (bookmark_id, owner_id, local_title, parent_id, creation_date, folder_p, closed_p)
   

2. Around line 41:

   ($bookmark_id, $user_id, '$local_title', [ns_dbquotevalue $parent_id], sysdate(), 't', 'f', 0)
   

   instead of

   ($bookmark_id, $user_id, [db_postgres_doubleapos_null_sql $local_title]', [ns_dbquotevalue $parent_id], sysdate(), 't', 'f')
   

3. Around line 57:

   # bm_set_in_closed_p $db $user_id
   

   instead of

   bm_set_in_closed_p $db $user_id
   

The revised edit-bookmark-2.tcl file

# edit-bookmark-2.tcl,v 3.0 2000/02/06 03:35:21 ron Exp
# edit-bookmark-2.tcl
#
# edit a bookmark in your bookmark list
#
# by aure@arsdigita.com and dh@arsdigita.com

set_the_usual_form_variables

# local_title, complete_url, bookmark_id, parent_id, return_url

validate_integer bookmark_id $bookmark_id

set user_id [ad_verify_and_get_user_id]
ad_maybe_redirect_for_registration

set db [ns_db gethandle]

# start error-checking
set exception_text ""
set exception_count 0

if {(![info exists bookmark_id])||([empty_string_p $bookmark_id])} {
    incr exception_count
    append exception_text "<li>No bookmark was specified"
}

# make sure that the user owns the bookmark
set  ownership_query "
        select count(*)
        from   bm_list
        where  owner_id=$user_id
        and bookmark_id=$bookmark_id"
set ownership_test [database_to_tcl_string $db $ownership_query]

if {$ownership_test==0} {
    incr exception_count
    append exception_text "<li>You can not edit this bookmark"
}

# return errors
if { $exception_count> 0 } {
    ad_return_complaint $exception_count $exception_text
    return 0
}

if { ![info exists parent_id] || [empty_string_p $parent_id] } {
    set parent_id "null"
}
ns_db dml $db "begin transaction"

# ### not sure why I had to change this section, but it didn't work ###
# if the bookmark to edit is a folder, complete_url won't be defined
# if { ![info exists complete_url] } {
# if { [empty_string_p $complete_url]||![info exists complete_url] } {

    # this is a folder so edit its name
#     set sql_update "
#          update  bm_list
#          set     local_title = '[DoubleApos $local_title]',
#                  private_p = '$private_p',
#                  parent_id = $parent_id
#          where   owner_id = $user_id
#          and     bookmark_id = $bookmark_id"
#    set sql_update "
#         update  bm_list
#         set     local_title = '$local_title',
#                 private_p = '$private_p',
#                 parent_id = $parent_id
#         where   owner_id = $user_id
#         and     bookmark_id = $bookmark_id"
#    ns_db dml $db $sql_update

# } else {
if { [info exists complete_url] } {

    # entry is a bookmark - need to update both name and url

    set host_url [bm_host_url $complete_url]

    # check to see if we already have the url in our database
    set url_query "select url_id
                   from   bm_urls
                   where  complete_url = '[DoubleApos $complete_url]'"
    set url_id [database_to_tcl_string_or_null  $db $url_query]

    # if we don't have the url, then insert the url into the database
    if {[empty_string_p $url_id]} {
        set url_id [database_to_tcl_string $db "select bm_url_id_seq.nextval from dual"]
        ns_db dml $db "
        insert into bm_urls
        (url_id, host_url, complete_url)
        values
        ($url_id, '[DoubleApos $host_url]', '[DoubleApos $complete_url]')"
    }
# have added the url if needed - now just update the name

    set sql_update "
        update  bm_list
        set     local_title = '[DoubleApos $local_title]',
                url_id = $url_id,
                private_p = '$private_p',
                parent_id = $parent_id
        where   bookmark_id = $bookmark_id"

    ns_db dml $db $sql_update
} else {
    # this is a folder so edit its name
    set sql_update "
         update  bm_list
         set     local_title = '[DoubleApos $local_title]',
                 private_p = '$private_p',
                 parent_id = $parent_id
         where   owner_id = $user_id
         and     bookmark_id = $bookmark_id"
   ns_db dml $db $sql_update

}

# the bookmark_id's are tagged by a "before update" trigger on bm_list
# -- and they end up as v_updated_ids in bm_list_pkg

# just checking to make sure we have some updates to do...
set id_count [database_to_tcl_string $db "
   select count(*)
   from bm_list_pkg
   where v_num_entries > 0
   and v_updated_ids is not null"]

set i 0

# ### for some reason, I had to do the other selects in a different loop ###
# ... so, in this loop, I collect everything into arrays
if {$id_count > 0} {

  set selection [ns_db select $db "
     select distinct v_updated_ids, coalesce(parent_id, -1) as parent_id,
       case when parent_sort_key is null then local_sort_key
            else parent_sort_key || cast(local_sort_key as varchar(3)) end
         as parent_sort_key,
       folder_p
     from bm_list_pkg, bm_list
     where v_num_entries > 0
     and v_updated_ids = bookmark_id"]

  while {[ns_db getrow $db $selection]} {
    set_variables_after_query
    set v_upd_ids($i) $v_updated_ids
    set new_parent_id($i) $parent_id
    set old_parent_sort_key($i) $parent_sort_key
    set is_folder($i) $folder_p

    incr i
  }
  while {$i > 0} {
    incr i -1

    if {$new_parent_id($i) > -1} {
      set v_parent_sort_key [database_to_tcl_string $db "
        select case when parent_sort_key is null
          then local_sort_key
          else parent_sort_key || cast(local_sort_key as varchar(3)) end
        from bm_list where bookmark_id = $new_parent_id($i)"]

      set v_last_sort_key [database_to_tcl_string $db "
        select max(local_sort_key) from bm_list
        where parent_id = $new_parent_id($i)
        and bookmark_id != $v_upd_ids($i)"]

      ns_db dml $db "update bm_list set parent_sort_key = '$v_parent_sort_key',
        local_sort_key = new_sort_key('$v_last_sort_key')
        where bookmark_id = $v_upd_ids($i)"

    } else {

      set v_last_sort_key [database_to_tcl_string $db "
        select max(local_sort_key) from bm_list where parent_id is null
        and bookmark_id != $v_upd_ids($i)"]

      ns_db dml $db "update bm_list set parent_sort_key = null,
        local_sort_key = new_sort_key('$v_last_sort_key')
        where bookmark_id = $v_upd_ids($i) and parent_id is null"

    }
    set v_new_sort_key [database_to_tcl_string $db "
      select case when parent_sort_key is null then local_sort_key
        else parent_sort_key || cast(local_sort_key as varchar(3)) end
      from bm_list where bookmark_id = $v_upd_ids($i)"]
    set len_old_sort_key [string length $old_parent_sort_key($i)]
    ns_db dml $db "update bm_list
      set parent_sort_key = '$v_new_sort_key' ||
        substr(parent_sort_key, $len_old_sort_key+1,
          length(parent_sort_key)-$len_old_sort_key)
      where parent_sort_key like '$old_parent_sort_key($i)%'"
  }
}

# ### need to delete all the entries "tagged" by the update trigger
# ### now that we're done updating sort keys
ns_db dml $db "delete from bm_list_pkg where v_num_entries > 0"

# ### I tried to do this as a Tcl proc in tcl/bookmarks-defs.tcl, ###
# ### but no luck on that (so far)...
# bm_recalc_sort_keys $db $user_id

# ### this may need to be changed, at some point...
bm_set_hidden_p $db $user_id

# ### the first of these is my modification of bm_set_in_closed_p, ###
# ### which, as you can tell, does only ONE folder
# ### forget why I had to comment it out, but it's NOT relevant to
# ### this part of the bookmark module
# ### (bm_set_in_closed_p is commented out because it didn't work)
# bm_set_one_in_closed_p $db $user_id $bookmark_id
# bm_set_in_closed_p $db $user_id

ns_db dml $db "end transaction"

# send the user back to where they came from before editing began
ns_returnredirect $return_url

Changes in insert-one-2.tcl and insert-one.tcl

($bookmark_id, $user_id, $url_id,'[db_postgres_doubleapos_null_sql $local_title]', [ns_dbquotevalue $parent_id], sysdate())"

($bookmark_id, $user_id, $url_id,'$local_title', $parent_id, sysdate())"

In insert-one.tcl, looks like for some reason I had to omit an extra < /ul> tag, so that the following:

    If this is correct, choose which folder to place the bookmark in:
    <ul>
    <table>
    <tr>
    <td>
    [bm_folder_selection $db $user_id $bookmark_id]
    <p>
    <input type=submit value=Submit>
    </td>
    </tr>
    </table>
    </form>
    </ul>"

    If this is correct, choose which folder to place the bookmark in:
    <ul>
    <table>
    <tr>
    <td>
    [bm_folder_selection $db $user_id $bookmark_id]
    <p>
    <input type=submit value=Submit>
    </td>
    </tr>
    </table>
    </form>"

I'll be trying your code out on our company intranet in the next couple of days.  I'll post how it goes.

I guess we should've posted it in News, too.

Ben's right that OpenACS 4.1 will help a lot - modules will be individually downloadable, and those that aren't ported simply won't exist in the release tree.