General code and @todo clean-up

The patch contains lots of minor clean-ups that don't really deserve a separate issue. If something needs more discussion, I can remove it from the patch and open a separate issue...

- Removes many outdated @todo's
- Unifying on 'newsletters' for list of newsletter categories and also unifying on using #tree. This allows to simplify the code and requires some changes in the test code.
- Fixes some @todo's (e.g. by adding missing documentation)
- Moved node tab page/form callbacks to admin.inc
- moved api.php to the top-level folder. This isn't an include file, this is documentation.
- Coding style fixes (e.g. "Implements" instead of "Implementation of")
- Removed a number of functions that aren't used anymore.
- Removed theme functions for newsletter body/footer. There is imho no need to have theme functions *and* templates and it will lead to confusion what is actually used if it's not overriden (I think the theme functions were actually used, not the templates).

Let's see what the testbot has to say.

Ok, forgot to add the 'template' key to hook_theme() to tell the theme system to use the templates and not look for theme functions.

This one should pass.

That was actually based on an outdated version, this one should be better.

#5: correct_cleanup_with_updated_hook_theme.patch queued for re-testing.

Re-rolled the patch.

Actually detected a bug in the new code from #841966: include simplenews-newsletter-footer.tpl.php in hidden newsletters that didn't occur due to the duplicated theme functions. Fixed in a separate commit and re-rolled.

And next try.

About $node->simplenews, I was actually thinking about dropping that that completely and simply relying on simplenews_newsletter_load($node->tid) when we need "newsletter information". Which in most cases is just the tid. So maybe a function like simplenews_get_category($node) or something similar that actually gets it directly from the taxonomy reference field. That would also make it easier to remove the redundant column from the simplenews_newsletter table? I'll open a follow-up once I commited this. Probably tomorrow in the train or so...

Yes, simplenews_get_subscription() is a nasty one. It basically checks three keys (mail, snid, uid) but actually only one of them and strictly in that order, so it's pointless to pass in an account object that has mail and uid set as there always is a mail when there is a uid so uid is never checked for. Re: core patterns, core actually uses something_load_by_something(), e.g. user_load_by_name(), so maybe we should actually add simplenews_subscriber_load_by_mail() and simplenews_subscriber_load_by_uid() (snid is already covered directly...by _load()), make simplenews_subscriber_load() return the same information as get_subscription and drop that one? Currently we have two kinds of subscriber objects which aren't the same thing.. Will open a follow-up for that one as well..

It's always hard in huge issues like this one to stop at some point and don't clean up forever.. there's still a lot of potential for doing so... :)

Commited.

Follow-ups:
- #1392290: Remove $node->simplenews and rely on simplenews_newsletter_node($node->nid)
- #1392294: Replace the simplenews_get_subscription() with simplenews_subscription_load_by_mail/uid()
- #1392314: Cleanup simplenews.api.php and create initial api doc groups