Port Upgrade Status to D7

Hm. This is harder than I anticipated because I don't quite follow the logic of what functionality in Update Status module this overrides and why, and the D7 code in some places looks quite a bit different (queues and whatnot). I will now attempt to have a conversation with myself to see if that helps. ;)

First, let's start by comparing Update Status 6.x with Upgrade Status 6.x, as well as Update Status 7.x with Update Status 6.x.

Sorry, that first one was a stupid diff. :)

When reading that diff, the "+" lines are important, since they're in Upgrade Status module. The "-" signs represent stuff that's only in Update Status module. Upgrade Status uses a couple of conventions, which are nice:

    // US: Blah Blah Description.

Comments that begin this way are places where the upstream code has been modified for Upgrade Status, along with why we're doing that.

#      foo();

Lines that begin with a # sign are the same, but commented-out lines that are not relevant to Upgrade Status module.

Let's start with...

update.compare.inc

Here, we're leaving all of the logic about actually getting the project data alone (i.e. update_get_projects(), _update_process_info_list(), update_get_project_name(), update_process_project_info(), update_project_cache(), and update_filter_project_info() are left alone), because this logic is the same regardless of what results are actually shown in the table.

Where we start overriding things is in upgrade_status_calculate_project_data() (formerly, update_calculate_project_data()). This makes a few logic modifications that are relevant, namely:

• Upgrade Status, unlike Update Status, doesn't care about defending users against unpublished, revoked, and unsupported projects, so code related to that is commented out. Same with code regarding dev snapshot special handling, and security updates.
• Update Status makes the assumption that if you have a 7.x-1.x version if your module, but 1.x is not found in "recommended major versions," (because you should be using 7.x-2.x instead) that you are using an unsupported project and will flag a warning about it. However, since module authors may very well port from a 7.x-2.x to a 8.x-1.x, that logic is nullified here.
• Update Status will stop when it's found the existing version, but Upgrade Status needs to keep going because the numbering scheme may be different between major versions. (?)
• Upgrade Status defines its own project statuses: UPGRADE_STATUS_DEVELOPMENT (there's a development release), UPGRADE_STATUS_STABLE (there's a stable release), and UPGRADE_STATUS_CORE (it's been moved into core). The logic around exposing Update Status-specific logic is therefore commented out in favour of this.
• Finally, there's a helper function upgrade_status_moved_into_core() with an enormous switch statement with that information, along with notes as to specific 'gotchas'.

Margh. One more try at that bloody diff. :P

update.fetch.inc

Here we fork a number of functions:

• update_manual_status() — A menu callback to trigger the project fetching outside of cron. We need it to call Upgrade Status's refresh function, and go to Upgrade Status's reports page, hence the (minor) hacks.
• _update_refresh() - Logic to fetch update data from Drupal.org. Upgrade Status does its own fetching of project data, separate from Update Status's. Therefore, all of the variables in _update_refresh() are changed from "update_foo" to "upgrade_status_foo": upgrade_status_project_projects (cache), upgrade_status_project_data (cache), upgrade_status_available_releases (cache), upgrade_status_check_frequency (variable), upgrade_status_last_check (variable), upgrade_status (watchdog log).

  Additionally, there's what looks like some handy debug code, as well as a timeout workaround we can apparently remove in D7.

• _update_build_fetch_url() - Logic to create the URL with which _update_refresh() refreshes its updates (heh). We don't want to pass a $site_key variable to Drupal.org so as not to distort project usage statistics, so we nullify that code. Also, where the version of the project in the Drupal.org URLs is hard-coded in Update Status to the current project version (e.g. 7.x in the case of a 7.x Drupal core installation), in Upgrade Status, it's a variable based on the selection in the form. Its default value is UPGRADE_STATUS_CORE_VERSION, which is +1 from the current core version — so "7.x" in the case of the 6.x version of the module, and it will be "8.x" in the case of the D7 port of Upgrade Status.

Things we leave alone in this file are:

• _update_get_fetch_url_base() — the "http://updates.drupal.org/release-history" part of said URL, which doesn't change.
• _update_cron_notify() — we don't do anything with e-mail notifications in Upgrade Status.
• The update_xml_parser class which processes d.o's XML feed. This works on the same data regardless.

So basically, what I take away from this is anything required to make _update_refresh() work in D7 needs to be forked into this module during the port.

update.report.inc

All parts of this file get forked in some way. Namely:

• update_status(): This is the menu callback for the report. It makes 3 changes:
  1. Only check cached project data from upgrade_status_get_available(), don't retrieve it on viewing the report. (this may or may not be good; it caused problems on my site I set up to track D7 ports from time to time when cron would fail).
  2. Calls Upgrade Status's version of update_calculate_project_data() and return theme('update_report', $data);
  3. In-lines the _update_no_data() function, which is really just a string (it is indeed weird that there is a whole separate function just for this... might be worth an upstream novice issue).
• theme_update_report() Various modifications to this report, which include:

  - Different stuff in the header, including a core version form, which is defined later on in upgrade_status_core_version_form() and upgrade_status_core_version_form_submit().
  - Makes "Drupal" the first project in the list; hides Upgrade Status.
  - Remove logic about notifications.
  - Add some extra divs to support Upgrade Status-specific "compact layout" styling. Mainly this involves making the table rows shrunk down and expandable by default.
  - Adds Upgrade Status's status constants to the appropriate table row styles: UPGRADE_STATUS_STABLE / UPGRADE_STATUS_CORE (ok / green), UPGRADE_STATUS_DEVELOPMENT (warning / yellow), UPGRADE_STATUS_CORE with a note of some kind (warning / yellow), unknown (red / not ported).
  - If it can't find an upgraded version of any kind, it uses the current project's data so at least something can be printed in the table.
  - Pull in Upgrade Status's CSS/JS instead of Update Status's, and rename classes accordingly.

• theme_update_version() is also overridden, but it's not clear why as there are no other hacks other than the name change. I'll probably undo that during the port.

And finally...

update.module

• All of the existing UPDATE_* constants are removed, in favour of UPGRADE_STATUS_* constants. These have been mentioned already previously, but to recap they are: UPGRADE_STATUS_CORE_VERSION (7.x), UPGRADE_STATUS_DEVELOPMENT (1000), UPGRADE_STATUS_STABLE (5), UPGRADE_STATUS_CORE (5000). I could not begin to explain why the numbering scheme that was used was used, other than 5 is the same as UPDATE_CURRENT (the "a-ok" status in Update Status module), so at least that one makes sense.
• For hook_help(), it just pulls in the contents of README.txt dynamically. Tricksy. ;) All of the other help blurbs that Update Status litters everywhere to scream OH MY GOD SECURITY ARRRRGHHH are removed.
• In hook_menu(), we only declare two entries: one to the report (admin/reports/updates/upgrade / upgrade_status_status()) and one to the manual status check (admin/reports/updates/upgrade/check / upgrade_status_manual_status()).
• hook_theme() declares upgrade_status_report and upgrade_status_version.
• We override update_get_available() and update_refresh() to point at _upgrade_status_refresh() instead.
• All code about requirements checking is ignored, as well as hook_cron(), hook_form_alter() (to make Update Status module run on the modules/themes page), and the aforementioned silly _update_no_data() function. Additionally, everything related to caching (the entire update_status_cache defgroup) are left alone, as well as anything related to notifications (hook_mail() and _update_message_text()). There's also a _update_project_status_sort() that's left alone. Ah-ha. That probably explains the aforementioned weird-ass numbers. :)

Ok. That helps. I think I might have a shot at this now.

Now a re-upload of the 6.x -> 7.x Update Status diff, since Git has made me rusty at passing command line ops to diff, so I forgot -p. :P~

Now some notes in reviewing the 6.x -> 7.x Update Status diff. These are not complete, just some things that stood out.

- In general, string concatenation rule changes " . " vs. ". " and some coding standard fixes add a bunch of annoying noise to this diff.
- In general, there's also a lot of added code around support for downloading new/updated modules and themes, all of which is irrelevant to Upgrade Status.
- time() is now REQUEST_TIME.
- All theme function arguments are now array indexes in "variables" (not "arguments")
- update_calculate_project_data() is now split into two functions: update_calculate_project_data() and update_calculate_project_update_status().
- There are some additions to update.css we might need to take into account.
- Update refreshing is now done via batch job, which sends fetch tasks to a queue. This is a big change. I'm not sure how much of that I'll need to fork yet, but possibly all of it. If so, I'll need the code from update.install that establishes such a queue.
- Version is still not passed into _update_build_fetch_url() so we'll need to continue to fork that.
- That icky custom XML parsing class is removed in favour of SimpleXML. Long live PHP 5!
- There's some kind of crazy code in hook_init() which i don't think we'll need.
- There's now support for tracking which sub-modules/sub-themes are enabled/disabled. Handy.
- Release labels (Revoked, Insecure, etc.) got pulled out into a theme_update_status_label(). We'll want to do something similar for Upgrade Status.
- There's now a toggle to check for disabled modules in core. Nice! This will make creating a contrib dashboard more straight-forward. :) For 6.x -> 7.x I needed Update Status Advanced Settings module to do that.

Hiding irrelevant crap.

Adding a checklist of sorts into the issue summary. :)

Awesome, thanks for the guidance!

Agreed splitting up the commits would be good to do this time. In almost all of the places I had questions, git blame led me to a single by sun: Revamped entire module and synced with Update module commit, which wasn't able to answer them. Your proposed plan of attack sounds good.

For D8, there's #1987888: Convert update_manual_status() to a new style controller (which is more like "refactor the entire Update Status module to be OO" than what it says on the tin) which I'm probably at least semi-qualified to review now ;) and that would hopefully allow for what you're suggesting. If I'm feeling extra plucky, I might even try a D8 port after that patch gets committed, but let's see how the D7 port goes first. :P

Adding recommended plan of attack to the issue summary.

Upgrade Status upgrade update. ;)

And another one!

Baby's awake, so the rest of this is going to have to wait. :)

Next steps:
- Get the core version selector appearing on the report.
- Go through both the remainder of report.inc and compare.inc and update "update" to "upgrade_status" where appropriate.
- Make the remainder of the "// US" hacks in all files.
- Figure out how to get the newfangeldy CSS/JS working. (I might punt on this, since I throw out the UI and re-create it anyway in https://drupal.org/sandbox/webchick/1223824.)
- Possibly add tests? I'll take a gander at how Update Manager's tests are done and if they could be applicable here.

Ok, now done with the basic port, with the exception that I kicked the compact layout stuff to a separate issue at #2172127: Port compact layout styling to D7 since my CSS/JS skills are not up to snuff. :\

Now it's time for everyone's favourite!

From there, simply try to run, use, and fix it in a trial & fail fashion ;)

;)

Btw, that should not be read in any way that the module is working in 7.x. ;) Just that all the modifications are there (or I think so anyway). In order to get the module actually working, I'm starting with #2172189: Add automated tests for Upgrade Status module.

Woohoo! :) It's working! :D Definitely not as pretty as the 6.x version, but the data is all there!

I'm sure there are bugs that will be found, and #2172239: Flesh out list of modules that were moved to core in 8.x definitely needs to be done, but I think I can officially mark this fixed and get a dev release rolled so others can test. :)

Fixing version now that there's a dev release for 7.x-1.x.

...and now I will stop spamming all inboxes everywhere. :) Thanks, this was really fun!