Comment 3 for bug 1895695

I see Terran is looking at this, but I saw thought I would offer some feedback about the general structure of the code.

With some notable exceptions, the OpenILS::Application Perl module namespace is generally reserved for modules (and module groups) that provide an OpenSRF application implementation. I would recommend the contents of the OpenILS::Application::EResourceLinkClick package be moved into the OpenILS::WWW namespace, and not inherit from OpenILS::Application, since it's not meant to register OpenSRF methods. In fact, as it doesn't have any overlapping sub names with the parallel OpenILS::WWW::EResourceLinkClick module, it could be absorbed there quite nicely, I think.

A couple schema notes:

  - The IDL says "clicked_at" is required, but it has a DEFAULT. This may be debatable, still, but I would lean towards not marking anything that has a database default.
  - The course column on action.eresource_link_click_course is NOT NULL and is meant to point to a course id, but does not have an FKEY. I would recommend the reverse of that -- allow it to be NULL, add the FKEY, and if we want to be able to remove the course, set the FKEY to ON UPDATE CASCADE ON DELETE SET NULL.

Otherwise, it all looks good from a code read.