Drupal 6 port, offer to co-maintain

hi cottser - thanks for the code and many patches! I hope to review these in the near future.

I've asked another potential co-maintainer to review the patches so I guess we'll go from there :)

Hi,

Is there a bug in the d6 port but when I went through the code and functionality of d7 version, it has the same bug.

In case the admin navigates to http://example.com/user/2/login_history (I am taking an example for a user with uid 2). In this case the login history of the user with uid should be displayed but the results show the login history of all the users(attached screen shot would give a better overview). The user is test in this case, but the results show the login history of admin too. The same bug is present in d7 version too.

Apart from that, "_" should be avoided in menu items, instead it should be replaced with "-". So the menu item would be

/**
 * Implements hook_menu().
 * Define menu items and page callbacks.
 *
 * @return
 *   An array of menu items.
 */
function login_history_menu() {
  $items = array();
  $items['admin/reports/login-history'] = array(
    'title' => 'Login history',
    'description' => 'Shows previous login information for site users. Useful for troubleshooting and monitoring.',
    'page callback' => 'login_history_report_callback',
    'access arguments' => array('administer users'),
    'file' => 'includes/login_history.pages.inc',
  );
  $items['user/%user/login-history'] = array(
    'title' => 'Login history',
    'description' => '',
    'page callback' => 'login_history_report_callback',
    'page arguments' => array(1),
    'access callback' => 'login_history_access_user_history_page',
    'access arguments' => array(1),
    'type' => MENU_LOCAL_TASK,
    'file' => 'includes/login_history.pages.inc',
  );
  return $items;
}

There is a coding standard error on line# 87 in login_history.pages.inc, else statement should begin on a new line.

Thanks,
Ashish

Sorry, wrong screenshot attached. The above one is for d7, please find the attached screenshot for d6.

So, @Cottser proposed becoming a co-maintainer in this issue and @a_thakur proposed it to me via email.

I'm inclined to add both of you as maintainers. Let me know if you have any more ideas or concerns on that.

Thanks @greggles for considering us to co-maintain the module.
I think views integration would be good initiative which was also raised in #1402266:Statistics and maybe views integration .

@Cottser: Could you please review my sandbox module: User Track which has views integration and it also takes into consideration failed login. I think some of the features can be merged here.

@greggles: I would be great in case you permit us to go ahead with views integration and other features which are implemented in my sandbox module.

Thanks,
Ashish

@Cottser - what do you think? I'm still inclined to add you both as maintainers?

@Cottser: It would be great in case look the User Track module in depth: code + functionality. As I believe I have been really careful while developing that module and has all the drupal coding standards expect at two places where the line exceeds 80 characters. All other issues like per user login history and other permissions are taken care of.

One thing I could think of is that we could maintain a new branch(the users would not be suggested to use this branch). where both of us could push the code and do a peer review of the code as well as the functionality and once we are sure enough the code is bug free we could merge the code to rc or the stable branch.

@Cottser: I hope you have had time to look at User Track in depth.

@greggles: I would be really great in case you could too, have a look the code and give your feedback. I think the method suggested in comment #10 is a good idea to go forward.

@Cottser, you're now a co-maintainer. Thanks!