Problem/Motivation

There is no mechanism for reporting on installed versions of libraries and checking for updates like Drupal core does for modules and themes. Thus the job is done inconsistently by modules that attempt it. For example, jQuery UI used to add an item to the status report with the installed version. WYSIWYG has it's own report-like interface. HTML Purifier used to poll the vendor web site and display a message on admin pages when it was out-of-date. But there's no single place to go to see the installed versions of all libraries registered with the system with latest available versions for each.

Proposed resolution

I propose the addition of a two things:

  1. A report, similar to Drupal core's "Available updates", that displays version information for all libraries registered in the system with links to vendor web sites and such.
  2. A mechanism for checking for and alerting administrators to available updates, probably a status report item and optional email notifications, like core does.

I think these would improve the site builder/administrator experience and help keep sites more stable and secure.

Remaining tasks

  1. Get feedback on the proposal.
  2. Agree upon a solution, including API additions.
  3. Write a patch implementing the solution!
  4. Write tests.
  5. Update documentation.

User interface changes

TBD.

API changes

TBD.

Original report by tstoeckler

This patch implements a UI for Libraries at /admin/reports/libraries.
The code can probably be cleaned up a bit, but I guess the basic layout/design needs to be decided upon first.
This depends on #719896: Add a hook_libraries_info().
Screenshot attached. I just enabled the test module from #719896: Add a hook_libraries_info() and visited /admin/reports/libraries and that's what you see.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

OH!

There has been some discussion around Wysiwyg's UI already: #487690: Improve installation instructions

tstoeckler’s picture

Great, will look into that. I hadn't looked into WYSIWYG at all so far. Initially I just wanted something to test #719896: Add a hook_libraries_info(), but then I liked it enough to roll a patch =).

tstoeckler’s picture

FileSize
14.23 KB

I know this isn't particularly high on the list currently, but I needed to do something soothing :)

This implements the concept of one centralized libraries overview at admin/reports/libraries (similar to drush libraries-list) and detailed per-library installation instructions.

Just turn on libraries_test.module and visit admin/reports/libraries.

I could of course provide screenshot, if needed.

drwierdo’s picture

a screenshot would be nice...

1kenthomas’s picture

Bump. Anything more on this? A UI seem critical to end-user troubleshooting...

RobLoach’s picture

Status: Active » Needs work

Patch broken... I think it should be the module defining the Library to provide the UI on this. I could be convinced other-wise though.

tstoeckler’s picture

Here's a re-roll (wow, this was still a CVS patch...).

I also attached some screenshots, which, at this point, are more important than the patch.
There are a bunch of things wrong with the patch code-wise, but I think we should settle on something desing-wise first, and then we can nit-pick the code.

I attached a screenshot of the Libraries overview, and two example screens of a library that is installed, and one that is missing. There are similar screens for libraries that e.g. have an unsupported version.

RobLoach’s picture

Don't really have time to test this right now, but just wanted to say THIS IS AWESOME!!! :-)

TravisCarden’s picture

Category: task » feature
Issue tags: +DX (Developer Experience)

At @sun's request, I've updated this issue with the summary I originally submitted as #1679840: Add mechanism for finding and reporting on available library updates. Hopefully it adequately represents the original efforts here. It probably broadens the scope a bit. If anyone has concerns, please voice them. Thanks for inviting me to the party. ;)

tstoeckler’s picture

Version: 7.x-1.x-dev » 7.x-2.x-dev
Issue tags: -DX (Developer Experience)

This is 7.x-2.x material at least. Also don't know why it's tagged DX.

Note that we have plans for completely revamping Libraries API for 7.x-3.x. The plans also include a UI, for automatically (!) installing Libraries directly, but that's a ways off.

Also, when we have the ability to download libraries directly, we'll have to rethink our UI efforts. Instead of a "passive" status report like the one proposed above, we'll probably need something rather resembling the Modules page.

Just leaving the note for now, not sure whether and how to tag this in light of the new plans.

klonos’s picture

Version: 7.x-2.x-dev » 7.x-3.x-dev

Seconding Rob's excitement from back in February: THIS IS AWESOME!!!

I'm surprised that I haven't spotted this issue earlier. Do you think early adoption from certain modules with willing maintainers would help in order to have actual testing grounds instead of having to use test libraries? Just asking because one comes to mind.

tstoeckler’s picture

Do you mean early adoption of 7.x-3.x? Sure, early adoption always helps.

With regards to this issue, as I tried to explain in #10, I think we need to take a step back and clearly define what we want. Since automatic installs from the UI is on the plate for 7.x-3.x, we really need to think how this all should look and what the use-cases are.

tstoeckler’s picture

Issue summary: View changes
jimmyko’s picture

So now we only plan to add this feature on 7.x-3.x branch? At this moment, 7.x-3.x do not have dev version but 8.x-3.x have. May I try to start this idea on 7.x-2.x?

Rob Holmes’s picture

FileSize
909.99 KB

Thought i should mention I have been using this UI proposal in my library pack module for quite some time now, it might be a good place to do the work separate from the libraries main module? https://www.drupal.org/project/library_pack

Pol’s picture

Status: Needs work » Needs review
FileSize
14.45 KB

Here's an updated patch for 7.x-2.x.

  • dadfcbb committed on 7.x-2.x
    Issue #819610 by tstoeckler, Pol: Provide a status report for library...
tstoeckler’s picture

FileSize
16.15 KB

Looked through the code and couldn't find anything standing out. I did a little bit of clean-up (see attached interdiff) and then committed it. One thing I did take out is the var_export() output. I'm not opposed to that in principle, but I think we should discuss the details of that before going with something. I added a @todo to that effect.

Keeping active, because we maybe want to improve on this a little bit, like clean-up some of the @todos I added. And also we should add some tests for the UI.

tstoeckler’s picture

Status: Needs review » Active
Pol’s picture

FileSize
17.3 KB

Here's an updated patch, there's still work to do, but I've updated these things:

- Adding a 'provider' column that indicates the library's provider. A module, a theme or an info file.
- Use one of these provider's name(instead of machine name) if any when needed.

Pol’s picture

Status: Active » Needs review

The last submitted patch, libraries_ui.patch, failed testing.

The last submitted patch, 3: libraries-ui.patch, failed testing.

tstoeckler’s picture

Version: 7.x-3.x-dev » 7.x-2.x-dev
Status: Needs review » Needs work

Ahh, interesting. @Pol, do you mind rolling the patch against the latest 7.x-2.x ? I already committed the previous code to the latest 7.x-2.x

Pol’s picture

Status: Needs work » Needs review
FileSize
7.23 KB

Here it is !

tstoeckler’s picture

Status: Needs review » Needs work
  1. +++ b/libraries.admin.inc
    @@ -20,9 +20,10 @@
    +  $modules_data = system_rebuild_module_data();
    

    Are you sure that's needed? If so, we should add a comment.

  2. +++ b/libraries.admin.inc
    @@ -39,11 +40,33 @@ function libraries_admin_overview(array $form, array &$form_state) {
    +    switch ($library['info type']) {
    +      case 'module':
    

    Since this appears both in the overview and in the status form I think it would make sense to add a little helper function for it.

  3. +++ b/libraries.admin.inc
    @@ -196,8 +237,7 @@ function libraries_admin_library_status_form(array $form, array &$form_state, $n
             // @todo Add support for themes by accessing $library['info type']
             if (!empty($library['module'])) {
               $form['status']['not_detected']['help']['#markup'] = t('If yes, the library information is corrupted. Contact the maintainer of the %module module with the following information:', array(
    
    @@ -257,7 +297,7 @@ function libraries_admin_library_status_form(array $form, array &$form_state, $n
               $form['status']['not_supported']['help']['#markup'] = t('If you are bound to the current version of the library, ask the maintainer of the %module module to provide support for version %version.', array(
    

    I think this would be a good time to resolve that @todo about themes as well.

  4. +++ b/libraries.module
    @@ -895,7 +895,7 @@ function libraries_menu() {
    -  $items['admin/reports/libraries/%'] = array(
    +  $items['admin/reports/libraries/%libraries'] = array(
    

    I'm not sure I agree with this change. That actually loads the library (i.e. includes the assets) which could have could have all sorts of side-effects. If we really want to use the menu system we should use a libraries_ui_load() function (or something with a better name) that does a libraries_detect() internally.

Pol’s picture

Status: Needs work » Needs review
FileSize
6.47 KB

Patch updated. It can be applied on 7.x-2.x now.

I will work on the remaining tasks soon.

Pol’s picture

A new patch fixing point 2.

Pol’s picture

This patch fix 1, 2 and 4. For the 3, I might need help on how to do.

Rob Holmes’s picture

FileSize
142.39 KB

Heres a screenshot of the new UI in action with a bunch of libraries installed. Looking good.

  • tstoeckler committed 68a567a on 7.x-2.x
    Issue #819610 by Pol, tstoeckler: Show the provider in the UI.
    
tstoeckler’s picture

Status: Needs review » Active

Sorry for the long turnaround. Finally took a look at the patch. Thanks a lot! In looking at the actual code I started refactoring libraries_admin_library_status_form() out into a couple of helper functions to make that less messy. I also adapted the position of the Provider row in the status information, it's now below the name and machine name, etc.

I also removed the system_rebuild_module_data() call again. Can you provide a clear example of what is broken without it? Or steps to reproduce or whatever?

I don't know if you want to keep this open for further refinements or if you want to close it and then open follow-ups if things come up. I'm fine either way, so marking "Active" for now, but feel free to mark fixed, if you want.

tstoeckler’s picture

Status: Active » Needs review
FileSize
64.43 KB

Wrote some tests for the new UI, let's see if the bot likes them.

Status: Needs review » Needs work

The last submitted patch, 32: 819610--tests.patch, failed testing.

tstoeckler’s picture

  • tstoeckler committed fea9637 on 7.x-2.x
    Issue #819610 by tstoeckler: Add tests for the Libraries UI.
    
tstoeckler’s picture

Status: Needs review » Active

So coming from #1884246: Drush download of defined libraries one thing we could do is display dependencies and variants in the UI. That is what drush libraries-list does, as well.

  • tstoeckler committed df0d700 on 7.x-2.x
    Issue #819610 by tstoeckler: Show variants and dependencies in the UI....
tstoeckler’s picture

Status: Active » Fixed

Implemented #36.

Since we've already been improving the UI in other issues, marking this one as fixed now. Please open new issues for improvements!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.