Lazy-load URLs into the database

I'm more than interested in collaborating!!!! (Nearly fell off the proverbial chair reading your message)

Patches welcome and if you want to be a co-maintainer I'd have no issue with that either (how could I - the flag module's your work ;) )

Perhaps we'll chat on IRC and sort out what needs to be done to get flag_page to a state where drupal.org can use it?!?

Here's a patch that adds:

1. Adds and uses the hash column for performance
2. Lazy loads the flag_page_data table so only flagged pages and entered
3. It doesn't add any extra menu items - it uses a content id of -1 to indicate it's a new url

Improved the way titles are handled by passing in the page title on the url

Still to do - break up the flag_page_data into two tables (flag_page_data_url and flag_page_data_user) - so that if users can't override titles we don't create unnecessary rows in the flag_page_data and the overridden data will be stored in flag_page_data_user.

This should be mean that on d.o flag_page_data_url is kept to a minimum size - i.e. maximum row count would be number of urls on the site rather than urls * users

This patch breaks flag_page_data into two tables flag_page_data_url and flag_page_data_user and makes the necessary changes to the views integration and contains all the improvements from #4 and #5.

Still to see what happens when anonymous flagging is enabled...

I've not read much into anonymous flagging... but I enabled session_api... set the page_flag to allow anon's... and it appears to work... have a look... http://dev.vit-al.co.uk/flag_page

Just enabled block level and normal caching mode... everything appears to work...

Tried it on aggressive and things fell apart.

Oops... nope it's not working when caching is enabled... however... the error is not that it thinks everything page is flagged - it never seems to get the remove flag action - and once you've added one url it's the only url you can add...

Will see what I can do...

Just enabled the javascript toggle... and it's working... when page caching is enabled... but not when block caching is enabled... any ideas as to why? I think that the flag / unflag block can't be cached so I did this in flag_page_block:

function flag_page_block($op = 'list', $delta = 0, $edit = array()) {
  if ($op == 'list') {
    $flags = flag_get_flags('page');
    foreach ($flags as $flag) {
      $blocks[$flag->name] = array(
        'info' => t('Flag page block for ') . $flag->title,
        'cache' => BLOCK_NO_CACHE);
    }
    return $blocks;
  }
  elseif ($op == 'view') {
    $block = array();
    $flag = flag_get_flag($delta);
    //Check if user hass access to flag
    if ($flag->user_access()) {
      $block['subject'] = $flag->title;
      $url = (isset($_GET['q'])) ? $_GET['q'] : variable_get('site_frontpage', 'node');
      $content_id = $flag->get_content_id($url);
      if ($content_id) {
        $block['content'] = flag_create_link($flag->name, $content_id);
      }
      else {
        $block['content'] = flag_create_lazy_page_link($flag->name);
      }
    }
    return $block;
  }
}

Can you guarantee the uniqueness of the sha1 generated id?

'cache' => BLOCK_NO_CACHE does work - just need to delete the row from the block table to get it to rebuild...

Okay now http://dev.vit-al.co.uk/flag_page has page caching (normal - not aggressive) and block caching turned on. Everything appears to work apart from the javascript that replaces the flag action with the unflag action...

Patch with latest changes... anonymous users and block caching...

Known bugs - for anonymous users the flag link will disable and not be replaced by an unflag link.

Any progress on this? What can be done to help remove blockers?

I tracked the issue with anonymous flags to bug a in the flag module when you use an underscore in a flag name - I raised an issue and submitted a patch http://drupal.org/node/904144

As far as I know I've implemented all of quicksketch's ideas to make the module scale for d.o - I will do a dev release of 6.x-2.x branch when I get a sec.

I've released a new version of the module 6.x-2.0 which has all the improvements recommended by quicksketch.

I've been doing some testing with flag_page and flag_note. In the 1.x series this combination works fine but in the 2.x series flag_note is note is not compatible with flag_page. I believe that this is due to this patch as flag is no longer referring to a pre-existing content id. This is reflected in the URL structure for flagging a node vs. flagging a page. The link for flag_page is not overridden to flagnote's menu item. Here are the examples:

flagnote/action/flag/bookmarks/5?destination=node%2F5&token=b5eca6c1b9af4c81e0c193c512a17d0f
flag/flag/bookmark_page/-1?destination=node%2F5&token=24db852edcd2f6b011cf999eef537ce3&title=Test

Any ideas? Time depending I may dig into this further myself. I've filed the bug at #999152: Flag note 2.x does not work with flag page 2.x.

As mentioned in #22 this patch introduced a compatibility problem between flag_note and flag_page. Removing the following code fixes the problem:

/*
 * Override get_link_type() function so flag_page module can create a
 * content_id only when necessary.
 */
function get_link_type() {
  $link_types = flag_get_link_types();

  // Override the default flag_flag_link by changing the module of ther selected link type.
  if (isset($this->link_type) && isset($link_types[$this->link_type])) {
    $link_types[$this->link_type]['module'] = 'flag_page';
    return $link_types[$this->link_type];
  }
  else {
    $link_types['normal']['module'] = 'flag_page';
    return $link_types['normal'];    
  }
}

From what I can see everything still works fine with this change but I can't be sure about other issues. Anyone care to comment on a way forward with this regression?

I'm moving to "needs work since this issue is still open despite the patch being committed. Let me know if any bugs should be new issues from here on out.

I think the best way to get round this issue will be to make the lazy-loading optional (and advisable on large sites). And check for the existence of the flag_note module and warn users. I've created a new issue to track this work #1017484: Compatibility problem between flag_note and flag_page 6.2

Setting this issue to fixed as the lazy loading capability works as expected in flag page 6.x-2.0

By overriding get_link_type() and assingning flag_page as module for all link_types flag_page is overriding href and destination attributes used by other flag modules.
As a result modules such as flag_note don't work as they are suppose to be working.

Here is the hook_flag_link implementation of flag_note

function flag_note_flag_link($flag, $action, $content_id) {
  $token = flag_get_token($content_id);
  return array(
    'href' => "flag/note/$action/$flag->name/$content_id",
    'query' => drupal_get_destination() .'&token='. $token,
  );
}

and below is the hook_flag_link implementation by flag_page

function flag_page_flag_link($flag, $action, $content_id) {

  $title = isset($_GET['title']) ? $_GET['title'] : drupal_get_title();
  $token = flag_get_token($content_id);

  if (drupal_is_front_page()) {
    $title = variable_get('site_name', '');
  }
  
  return array(
    'href' => 'flag/'. ($flag->link_type == 'confirm' ? 'confirm/' : '') ."$action/$flag->name/$content_id",
    'query' => drupal_get_destination() . '&token='. $token . '&title=' . rawurlencode($title),
  );  
}

If you look closely the difference between the two implementations is the callback URL "href". flag_note uses "flag/note/%/%/%" and flag_page uses "flag/%/%/%" . As a result once flag_page is activated control never reaches menu path "flag/note/".

I spent quite a it of time in trying to identify a way for passing additional attributes to hook_flag_links, however it doesn't seem to be possible without considerable changes to flag_preprocess_page().

function get_link_type() in flag_page.inc is resulting in other modules not working as they should, hence this function could be taken off till there is a better way.

Fixing status. This is done.