Last modified: 2014-02-12 23:47:44 UTC
I was browsing through some other code and noticed this: First, the comment says "Requires Extension:Echo". It actually requires [[mw:Extension:Thanks]], which in turn requires Echo. It then has a class_exists( 'MWEchoDbFactory' ) call. This tells you if Echo is installed, not if Thanks is installed. class_exists( 'ApiThank' ) will probably work. Then comes "MWEchoDbFactory::getDB( DB_SLAVE )". Echo has been written to be used with multiple backends. Even though DB is currently the only supported ones, that should not be depended upon (Gerrit change #92248 is an attempt to add a Redis backend). Echo doesn't really support grouping by category, this is probably something that should be done in the Thanks extension (storing the last Thank). That said, according to bug 49087 comment 4, "Thanks are intended to be ephemeral" so I'm not sure trying to look for the last one, regardless of how old it is, is a good idea.
Prioritization and scheduling of this bug is tracked on Mingle card https://wikimedia.mingle.thoughtworks.com/projects/mobile/cards/1384
Although misleadingly and confusingly this section of the profile actually only requires Echo... The only really issue here is if Thank is installed then uninstalled but Echo remains there will still be a Thanks section on some profiles. Thus I'll add an additional check since it is harmless to do so.
Change 95302 had a related patch set uploaded by Jdlrobson: Check Thank extension exists before rendering section on profile https://gerrit.wikimedia.org/r/95302
Change 95302 merged by jenkins-bot: Check Thank extension exists before rendering section on profile https://gerrit.wikimedia.org/r/95302
I don't think this is fixed. The code is still dependent upon the "Db" backend and should be abstracted. At the very least there needs to be a $wgEchoBackendName == 'Db' check.
Personally I think Echo itself should abstract its implementation away. Since Echo is not currently used with any other backends I'm reluctant to spend much more time on this personally when the main more pressing issue here has been fixed by checking for Thanks being enabled. When Echo does support other backends it should do a better job of surfacing 'APIs' that other extensions can feed into (as far as I'm concerned accessing Echo in the current way is a hack and a stopgap). I suggest raising a bug on Echo asking that other extensions have a safe way to extract this sort of information without caring about the backend and creating an enhancement request for mobile to support multiple backends.
Opened: bug 57043
(In reply to comment #6) > Personally I think Echo itself should abstract its implementation away. Since > Echo is not currently used with any other backends I'm reluctant to spend > much > more time on this personally when the main more pressing issue here has been > fixed by checking for Thanks being enabled. Echo already has this abstraction layer. The backend is exposed using the global $wgEchoBackend, which is an instance of MWEchoBackend. Accessing a user's notification can be done with ApiEchoNotifications::getNotifications (this can probably be improved). > > When Echo does support other backends it should do a better job of surfacing > 'APIs' that other extensions can feed into (as far as I'm concerned accessing > Echo in the current way is a hack and a stopgap). If it's a hack, then this bug isn't fixed. > > I suggest raising a bug on Echo asking that other extensions have a safe way > to > extract this sort of information without caring about the backend and > creating > an enhancement request for mobile to support multiple backends. I don't know what kind of information you want to extract, it would be better if you filed the bug yourself. This is the first case where another extensions is accessing the information Echo stores which is why there is no real internal API yet.
I did raise the bug - see bug 57043 - it asks for that internal API. MobileFrontend is hacking around a lack of it in this particular case.
