Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a views field handler for project properties from the database. #5820

Closed
4 tasks done
argiepiano opened this issue Oct 6, 2022 · 23 comments · Fixed by backdrop/backdrop#4229
Closed
4 tasks done

Comments

@argiepiano
Copy link

argiepiano commented Oct 6, 2022

Description of the need

Backdrop currently provides Views definitions to display module and theme information stored in the system database. Currently, it's possible to display theme/module filename, name, schema_version, status and type.

However, there is no way to display any of the information stored in the info column, which includes version, dependencies, backdrop version, tags, etc.

This is a request to add such a handler.

Proposed solution

Write a special field handler to display those values. Since the info column is a serialized array, there needs to be a special option definition and option form within the handler to select the specific setting to be displayed.

Alternatives that have been considered

None.

Draft of feature description for Press Release (1 paragraph at most)

Backdrop now includes a Views field handler to display setting information stored in the module and theme info files.


Minor release update:

  • Add a conclusion suitable for blog post + release notes
  • N/A Add Documentation label if the feature needs to be documented (or needs docs to be updated)
  • N/A Add Needs change record label if the the issue contains an API change.
  • Add Needs translation label if the the issue contains string translations.

Translated strings:

  • Project filename
  • Project name
  • The name of the module/theme/theme engine; e.g. node.
  • Project type
  • Project status'
  • Boolean indicating whether or not this module/theme/theme engine is enabled.
  • Project properties
  • Selected properties from the module/theme/theme engine\'s info file.

conclusion suitable for blog post

Backdrop now allows includes Views support for the system table. It's possible to generate a view of your site's enabled modules.

@argiepiano
Copy link
Author

PR submitted: backdrop/backdrop#4229

The PR adds a select box to allow the admin to select which setting from the info column to display. Not all settings are provided, and some will only return a value for modules (not for themes). Empty strings are returned if the setting doesn't exist.

@argiepiano
Copy link
Author

This feature request was prompted by work done for #5791. That issue provides a View to display enabled modules, but some of the information (like version) was not accessible, until this PR.

@argiepiano
Copy link
Author

Screen Shot 2022-10-06 at 6 30 05 PM

Screen Shot 2022-10-06 at 6 30 21 PM

@indigoxela
Copy link
Member

Tested in the Tugboat sandbox - works for me. 👍

@klonos
Copy link
Member

klonos commented Oct 7, 2022

This works great, thanks @argiepiano 🙏🏼

I've left several comments/suggestions in the PR, mainly about:

  • In Backdrop we do not have only modules/themes - we also have layout templates. We collectively call these "projects", so we can use that word instead of listing every type of project.
  • What you refer to as "settings" in comments and UI text is actually either "information" or "properties" (that's what we call the various entires in the .info files - see https://docs.backdropcms.org/creating-modules)

@klonos klonos changed the title Provide views field handler to access info column for modules and themes in system table Provide views field handler to access project properties from the info column in the system table Oct 7, 2022
@klonos
Copy link
Member

klonos commented Oct 7, 2022

...the "Dependencies" option outputs the machine name of the dependencies as a comma-separated list. Can we rename that option to "Dependencies (machine names)" and then add another "Dependencies" option that outputs the project names of the dependencies? (use system_rebuild_module_data() list_themes() etc. to retrieve that info)

@argiepiano
Copy link
Author

Thanks so much for reviewing, @indigoxela and @klonos. @klonos, thanks for clarifying the terminology! I was a bit lost with that.

I'll work on incorporating suggestions in the next couple of days.

@argiepiano
Copy link
Author

In Backdrop we do not have only modules/themes - we also have layout templates. We collectively call these "projects", so we can use that word instead of listing every type of project.

Layout template information is not stored in the system table, and therefore is unavailable to this handler. Also, existing Views definitions for the table system and all defined fields and filters in system.view.inc use the wording Module/Theme/Theme engine, so I felt that using that name was important for this new handler this PR. I guess we could change all of them to "Project" (which indeed is nicer than "Module/Theme/Theme engine"!)

Can you please clarify, in case I'm not understanding your suggestion? Thanks!

@argiepiano
Copy link
Author

argiepiano commented Oct 8, 2022

I just pushed a new, much improved version. I incorporated many of your suggestions. More testing, please!

I've added an "Other" input, that allows you to type the name of any additional property not included in the options array. This way you can possibly type new properties. And if necessary, this handler can be modified in the future to expand the options array.

I've added several other improvements for flat array properties like tags and dependencies. You can now specify the separator, or order/unordered list. Also dependencies give you the option of outputting machine names or human labels.

@indigoxela
Copy link
Member

@argiepiano many thanks for your work! 🙏 I just did a quick test this time - still works for me.

@argiepiano
Copy link
Author

argiepiano commented Oct 8, 2022

One question, why one common field with a select list rather than individual fields? Not saying I prefer it the other way, but I find Views fields seem to lean toward granular options. Any particular reasoning here?

Thanks @docwilmot. The reason it's done this way is that Views field handlers are abstractions of select queries. Since the information handled by this handler comes from a single column (info in the table system), and since info contains a serialized array with ALL properties in the info file, you can't really do a select query for a single property. You get the whole info column at once, which has to be unserialized.

That said, it is possible to create several "faux" handlers that use 'real field' => 'info' in the handler definition, and each individual handler could be made to retrieve the whole sql field info and then only display ONE specific property. That would mean creating about 11 or 12 different individual handlers...

We could go that route if it's preferred. The current approach also allows the user to specify a property using a machine name by selecting "Other"; this allows someone to display future properties, or properties that are not already included in the list.

@indigoxela
Copy link
Member

... it is possible to create several "faux" handlers that use 'real field' => 'info' in the handler definition...

Honestly, I don't see much benefit in that - it would only bloat the code with handlers that actually do the same re db query, but claim to do something different just by different rendering.

@argiepiano
Copy link
Author

argiepiano commented Oct 9, 2022

Just to add to my comment above: there is contrib precedent for an omnibus views handler that has select options like this one: Webform. The "Webform submissions data" handler is a single handler that allows the user to select specific "fields" to display. In Webform's case, I don't really like it, but because it's an ancient (pre-Field API) module, that's their workaround to display "fields" in Views.

In this handler's case, since the info properties are stable and well-defined, it's OK. Plus this handler will probably not be used as much as the webform one.

@jenlampton
Copy link
Member

jenlampton commented Dec 29, 2022

This PR is looking great, but I do think we need to change the label to "Projects" as recommended by @klonos. After that one-line change the PR looks RTBC!

I would prefer the shorter "Project properties". It will be consistent with our other uses of "Projects" in core and the documentation, where some project types may be excluded.

Alternate theme engines are not supported in Backdrop, so I worry that including that label here might be confusing for people. I also don't think the omission of some project types is really a problem. Including every type of project that might appear in the system table (profiles, etc) would make the label cumbersome and less usable, even though it may be more technically accurate.

@argiepiano
Copy link
Author

Thanks, @jenlampton. I've made the requested changes and fixed some style issues pointed out by phpcs. However, the latest PR had a phpcs failure I don't understand (phpcs not found ?).

@quicksketch
Copy link
Member

I gave this a code review and test and did not find any major issues.

One question that occurred to me: This exposes the scripts .info property to list JS files, but it does not expose the stylesheets .info property to list CSS files. stylesheets is a bit more complicated since it is a 2D array with the media type as the first level of nesting. I don't think this is a major bit of functionality though, so I would be completely fine with not including it. But if we don't have stylesheets; I think it might make sense to remove scripts as well.

@argiepiano
Copy link
Author

argiepiano commented Dec 30, 2022

@quicksketch, I've added stylesheets in the latest commit. The PR actually can handle 2D arrays (displayed as multilevel bullets - see image).

One aspect I did not hear about from @jenlampton: I followed her suggestion and changed "Module/Theme/Theme engine properties" to "Project properties". However, there are other system views handlers that still use "Module/Theme/Theme". My question: even though this is beyond this issue, should I change ALL labels to "Projects"?

Screen Shot 2022-12-29 at 8 09 41 PM

Screen Shot 2022-12-29 at 8 09 48 PM

@quicksketch
Copy link
Member

However, there are other system views handlers that still use "Module/Theme/Theme". My question: even though this is beyond this issue, should I change ALL labels to "Projects"?

I would be very much in favor of that change. The length of those titles is difficult to read. I would change to simply Project filename for example, but still list "Module/Theme/Theme engine" in the handler description so they show up when filtering the field list.

@argiepiano
Copy link
Author

@quicksketch, I've modified as indicated.

I'm wondering if a followup issue to create a filter would be in order?

@quicksketch
Copy link
Member

It would be neat, but filtering on a serialized column is not an easy task.

The new PR looks great. Looks ready to go to me.

@quicksketch
Copy link
Member

Merged backdrop/backdrop#4229 into 1.x for 1.24.0. Thanks @argiepiano et al!

backdrop/backdrop@3c5471d by @argiepiano, @indigoxela, @klonos, @jenlampton, @quicksketch and @docwilmot.

@jenlampton jenlampton changed the title Provide views field handler to access project properties from the info column in the system table Add a views field handler for project properties from the database. Jan 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants