stpaultim commented 2 years ago

Description of the need

We frequently ask folks who are having difficulties with Backdrop to share a list of module they are using. I was asked for this today.

In fact, this is not an easy thing to do and really it should be. Yes, it's easy to see all enabled modules on the admin/modules/list page. But, it's not easy to share that list in a forum post or Github issue. Not nearly as easy as it should be.

I would like a text only or bulleted list of my enabled modules that I can easily export or share in a forum post. I would assume this is pretty easy to provide..

I looked for an existing feature request but could not find one. I was a bit surprised. Maybe it already exists and I haven't found it. Or maybe there is an easy way to do this already that I'm not aware of.

Proposed solution

I'm going to suggest a report under the admin/reports menu that provides a list of all modules that can be filtered in any of the following ways:

1) All modules 2) All enabled modules 3) All enabled contrib modules 4) All enabled core modules

I think this should be available in the CORE UI because often the folks who need to share this information with someone do NOT have command line access to drush or Bee and may not be able to add a contributed module themselves.

Alternatives that have been considered

For Drupal 7 there is a Drush command for this, I'm not sure if it works on Backdrop. But, even if it does, many of the users that need this feature are not going have access to a Drush or Bee based solution. I think this should be possible in the UI.

Is there a Drupal or Backdrop contributed module that accomplishes this?

It looks like this Drupal 7 module might do the trick, but I have not tested it. https://www.drupal.org/project/enabled_modules

Based upon discussion in this issue, I've released a recipe that generates a view for this: https://backdropcms.org/project/enabled_modules_report_recipe

Additional information

Related idea - list of "unused" modules - https://github.com/backdrop/backdrop-issues/issues/5020 Contrib proof of concept: https://github.com/backdrop-contrib/unused

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

Backdrop CMS now includes a copy and paste debug report that can be used when reporting errors or seeking support in forums or chat. The new debug report includes server info, database version, jQuery info, a list of enabled modules and themes, and more. We expect that this report will make it much easier for site owners to get answers to technical questions, by making it easier for them to share the specifics of their site installation.

izmeez commented 7 months ago

Excuse me, I'm lost. Looking at the PR it looks to me like a plain text solution, not a view. Is that the case or what?

klonos commented 7 months ago

@izmeez yes, during the course of this issue it was initially proposed that we provide a views-provided report page, and @indigoxela provided a PR for that (then the same PR was revived later by @stpaultim). I counter-suggested a simple, plain-text implementation that would allow people to easily copy-paste the report in a format expected in places like GitHub messages, or text files uploaded to support tickets (for your developers or support engineers working in Drupal/Backdrop-specialized hosting companies etc.).

izmeez commented 7 months ago

Good, I'm glad things are as I understood.

Personally, I favor the views approach, https://github.com/backdrop/backdrop/pull/4298.

Not just because it illustrates the usefulness of views but also because of the future potentials. It might also then be cloned and modified.

izmeez commented 7 months ago

Unfortunately, it has conflicts that need to be resolved to test it.

klonos commented 7 months ago

I see merit in having a view-powered report (as I mentioned in an earlier comment here), however the views-based solution only provides a list of modules/themes. The custom, plain-text approach also provides the following:

Backdrop core version
Installation profile being used
PHP/Database server/Web server versions
Theme used in the frontend
Theme used in the admin backend
Information about the base theme if subthemes are being used
whether the admin theme is configured to be used when editing content or not

All that is information we usually ask people to provide in Zulip, forum.backdropcms.org, or in the various GitHub queues. Since the purpose this issue was filed was to provide an easy way to grab all that information for debugging/troubleshooting, I find the text-based solution to be a better fit and to be addressing the intended use.

klonos commented 7 months ago

...oh, I should add jQuery version and CKEditor versions to the list of items in the report. Will work on that next.

izmeez commented 7 months ago

Isn't the extra info what #5820 is to help with?

bugfolder commented 7 months ago

The text-based solution—with the extra information about core version, profile, jQuery, etc.—seems like a great time-saver if (a) something is going wrong, and (b) more than one person is eventually involved. Sounds like an 80% feature to me, ergo, good for core. (That's not 80% of sites—we hope—but 80% of users, at least, if they're regularly building sites or helping others that are doing so.)

klonos commented 7 months ago

Isn't the extra info what #5820 is to help with?

@izmeez no, that is different. That issue allowed grabbing additional information for "projects" (collective term we use for modules + themes+ layout templates) from their .info files, and storing them in the database and then using them in views. Previously, only a limited set of such information was available, and with that change merged we are now able to retrieve additional information, for each individual project though - not information about the server and what versions of software run there etc.

For example

the "Backdrop version" property added here is used to denote the version of backdrop core that a specific project is compatible with - that doesn't tell us the version of backdrop the site is using.
the "Base theme" property added here is used to denote the base theme that a subtheme is built upon - that doesn't tell us which themes are being used on a site as the frontend/admin.

klonos commented 7 months ago

@izmeez if you'd like an easy way to build a view with modules/themes, then I'd suggest the following:

Raise a new core issue to have such a view built and shipped with core. We could have that be disabled by default, and only provided as an example to demonstrate that that (generating a list of projects) is possible with core. I understand that it is easier for people to enable something that is already built but disabled, and then start tweaking that to their needs, rather than having to build that from entirely from scratch.
Request that something is built as a contrib recipe. That would be able to be installed as a regular module on the site, and I'm guessing that something like that will have less resistance (because it can be built in contrib and doesn't have to be included in core)
Request that some documentation is provided to guide people to build such a view, or perhaps contribute that yourself if you have the time/energy (we can help you with any technical problems).

klonos commented 7 months ago

Request that something is built as a contrib recipe.

...I'm sprinting with @stpaultim live over Jitsi, and he mentioned that a recipe already exists for a view that lists projects: https://github.com/backdrop-contrib/enabled_modules_report_recipe

stpaultim commented 7 months ago

I finally closed my views based PR. It would have required a rebase and I think Greg's PR is better. I'm going to mark this Works for Me!

klonos commented 7 months ago

Friendly reminder about this request I made earlier:

I am obviously advocating for this, so can I please get someone to remove the milestone candidate - minor label and add the actual milestone if you'd like to see this in the next minor release?

klonos commented 7 months ago

I've updated the PR to also report on jQuery and jQuery UI versions, as well as versions of text editors being used, and in which text formats. Here's a sample output of what it looks like:

Backdrop CMS:                 1.28.x-dev
Installation profile:         standard
PHP version:                  8.3.6
Database server:              10.11.7-MariaDB-1:10.11.7+maria~ubu2204-log
Web server:                   Apache/2.4.57 (Debian)
jQuery version:               3.7.1
jQuery UI version:            1.13.2

CKEditor 5, 40.2.0            used in: Another CKE5 format, Basic
TinyMCE 1.x-1.3.0             used in: test text format

Default theme:                Shasetsu (shasetsu) 1.x-1.1.0
 ⮑ base theme:                ⮑ Basis (basis) [core]
Admin theme:                  Seven* (seven) [core]
                              *configured to also be used when editing or creating content

admin_bar                     [core]
block                         [core]
ckeditor5                     [core]
color                         [core]
comment                       [core]
config                        [core]
contextual                    [core]
dashboard                     [core]
date                          [core]
dblog                         [core]
devel                         1.x-dev
devel_generate                1.x-dev
email                         [core]
entity                        [core]
field                         [core]
field_sql_storage             [core]
field_ui                      [core]
file                          [core]
filter                        [core]
image                         [core]
installer                     [core]
layout                        [core]
link                          [core]
list                          [core]
menu                          [core]
node                          [core]
number                        [core]
options                       [core]
path                          [core]
redirect                      [core]
search                        [core]
system                        [core]
taxonomy                      [core]
telemetry                     [core]
text                          [core]
tinymce                       1.x-1.3.0
update                        [core]
user                          [core]
views                         [core]
views_ui                      [core]

argiepiano commented 7 months ago

I'm getting this when running on a site:

Warning: Undefined array key "version" in system_get_debug_info() (line 2453 of /Users/XXX/Sites/localhost/demta/core/modules/system/system.admin.inc).

I'm using a custom theme whose base theme is another custom theme based on Basis, and therefore doesn't have a version.

Setting this back to NW

quicksketch commented 7 months ago

I really like the look of this PR! We probably need test coverage for it, even though it's just for debugging purposes. It would be pretty embarrassing to ask someone for their debug information only to have it not work.

I left some other feedback in the PR on how we don't need to add a new library_version key to hook_editor_info(): https://github.com/backdrop/backdrop/pull/4446#pullrequestreview-2026862362

klonos commented 7 months ago

Thanks for the review and testing @argiepiano and @quicksketch 🙏🏼 ...I'll work to address any feedback.

@quicksketch what would the tests look like? Test if the debug info page is loading and the text field is populated?

quicksketch commented 7 months ago

what would the tests look like?

Yeah, load the page and then confirm the text contents match against a predefined string would be sufficient.

olafgrabienski commented 7 months ago

@klonos Do you think it's possible to put a label or a heading at top of the modules list?

And what do you think about separating the modules list into groups? (Core, Contributed, Other) I think, often contributed modules are more interesting for debugging and support. Having module groups would make it easier spot them.

quicksketch commented 7 months ago

I updated https://github.com/backdrop/backdrop/pull/4446 with a few changes, I hope that's okay @klonos.

Added test coverage.
Separated modules by the categories: core, contrib, custom, and other. Determining if a module is contrib is done by presence of a /contrib/ directory or a project specified in the .info file (which is added by the packager).
I switched [core] to be the actual core value. Yes it's a bit redundant, but IMO that's better than needing to scroll up to the first line to find the core version.
The format of the returned value from system_get_debug_info() I changed from being $label => $value to array('label' => $label, 'value' => $value) because I encountered a problem where we may want the same line label multiple times (such as a row made of dashes to form an underline).

General question: Should we be passing any of these strings through t()? If the person is seeking help, they may want the strings in English (assuming they're getting help from Forums/Zulip/etc). On the other hand, they could always visit the page while in the English language. And even if it were translated, anyone familiar with the English version could (likely) understand it.

Example output:

Backdrop CMS:              1.28.x-dev
Installation profile:      standard
PHP version:               8.3.2-1+0~20240120.16+debian11~1.gbpb43448
Database server:           10.0.38-MariaDB-1~xenial
Web server:                Apache/2.4.56 (Debian)
jQuery version:            3.7.1
jQuery UI version:         1.13.2

CKEditor 5, 40.2.0         used in: Basic

Default theme:             Basis (basis) 1.28.x-dev
Admin theme:               Seven* (seven) 1.28.x-dev
                           *used when editing or creating content

Core modules               
------------               
admin_bar                  1.28.x-dev
block                      1.28.x-dev
ckeditor5                  1.28.x-dev
color                      1.28.x-dev
comment                    1.28.x-dev
config                     1.28.x-dev
contextual                 1.28.x-dev
dashboard                  1.28.x-dev
date                       1.28.x-dev
dblog                      1.28.x-dev
email                      1.28.x-dev
entity                     1.28.x-dev
entityreference            1.28.x-dev
field                      1.28.x-dev
field_sql_storage          1.28.x-dev
field_ui                   1.28.x-dev
file                       1.28.x-dev
filter                     1.28.x-dev
image                      1.28.x-dev
installer                  1.28.x-dev
layout                     1.28.x-dev
link                       1.28.x-dev
list                       1.28.x-dev
menu                       1.28.x-dev
node                       1.28.x-dev
number                     1.28.x-dev
options                    1.28.x-dev
path                       1.28.x-dev
redirect                   1.28.x-dev
search                     1.28.x-dev
simpletest                 1.28.x-dev
system                     1.28.x-dev
taxonomy                   1.28.x-dev
telemetry                  1.28.x-dev
text                       1.28.x-dev
update                     1.28.x-dev
user                       1.28.x-dev
views                      1.28.x-dev
views_ui                   1.28.x-dev

Contrib modules            
---------------            
basic_cart                 1.x-1.1.1
devel                      1.x-dev

Custom modules             
--------------             
my_custom_module           

Other modules              
-------------              
views_random_seed          1.x-dev

stpaultim commented 7 months ago

I love it. WFM!

We can make it better if we need to later, but this is already much better than what I originally envisioned!

I tested it on local environment. Added a bunch of contrib modules. Changed my JQuery version.

My site does not work, can anyone help me? ;-)

``` Backdrop CMS: 1.28.x-dev Installation profile: standard PHP version: 8.2.18 Database server: 10.11.7-MariaDB-1:10.11.7+maria~ubu2204-log Web server: nginx/1.24.0 jQuery version: 1.12.4 jQuery UI version: 1.13.2 CKEditor 5, 40.2.0 used in: Basic, Raw HTML Default theme: Opera (opera) 1.x-1.0.0-beta6 Admin theme: Seven* (seven) 1.28.x-dev *used when editing or creating content Core modules ------------ admin_bar 1.28.x-dev block 1.28.x-dev ckeditor5 1.28.x-dev comment 1.28.x-dev config 1.28.x-dev contact 1.28.x-dev contextual 1.28.x-dev dashboard 1.28.x-dev date 1.28.x-dev dblog 1.28.x-dev email 1.28.x-dev entity 1.28.x-dev field 1.28.x-dev field_sql_storage 1.28.x-dev field_ui 1.28.x-dev file 1.28.x-dev filter 1.28.x-dev image 1.28.x-dev installer 1.28.x-dev layout 1.28.x-dev link 1.28.x-dev list 1.28.x-dev menu 1.28.x-dev node 1.28.x-dev number 1.28.x-dev options 1.28.x-dev path 1.28.x-dev redirect 1.28.x-dev search 1.28.x-dev system 1.28.x-dev taxonomy 1.28.x-dev telemetry 1.28.x-dev text 1.28.x-dev update 1.28.x-dev user 1.28.x-dev views 1.28.x-dev views_ui 1.28.x-dev Contrib modules --------------- backup_migrate 1.x-1.0.26 entity_plus 1.x-1.0.21 entity_token 1.x-2.0.4 entity_ui 1.x-1.0.2 node_convert 1.x-1.2.0-beta2 rules 1.x-2.14.1 rules_admin 1.x-2.14.1 test_gateway 1.x-3.14.5 ubercart 1.x-3.14.5 uc_2checkout 1.x-3.14.5 uc_ajax_admin 1.x-3.14.5 uc_attribute 1.x-3.14.5 uc_authorizenet 1.x-3.14.5 uc_cart_links 1.x-3.14.5 uc_catalog 1.x-3.14.5 uc_credit 1.x-3.14.5 uc_cybersource 1.x-3.14.5 uc_file 1.x-3.14.5 uc_flatrate 1.x-3.14.5 uc_payment 1.x-3.14.5 uc_payment_pack 1.x-3.14.5 uc_paypal 1.x-3.14.5 uc_product_kit 1.x-3.14.5 uc_quote 1.x-3.14.5 uc_reports 1.x-3.14.5 uc_roles 1.x-3.14.5 uc_shipping 1.x-3.14.5 uc_stock 1.x-3.14.5 uc_tax_report 1.x-3.14.5 uc_taxes 1.x-3.14.5 uc_ups 1.x-3.14.5 uc_usps 1.x-3.14.5 uc_weightquote 1.x-3.14.5 views_bootstrap 1.x-3.5.1 views_bootstrap_demo 1.x-3.5.1 ```

olafgrabienski commented 7 months ago

Thanks for adding the headings to modules, @quicksketch, this looks really good!

As far as I know, we're only displaying enabled modules. Should that be mentioned? Unfortunately it looks a bit repetitive, but it's more clear.

edit: Actually, I think it's okay to repeat the term "Enabled", as the modules list can get quite long, and in that situation it doesn't seem so repetitive as I expected.

Enabled core modules               
--------------------               
admin_bar                  1.28.x-dev
block                      1.28.x-dev
ckeditor5                  1.28.x-dev
comment                    1.28.x-dev
config                     1.28.x-dev
contact                    1.28.x-dev
contextual                 1.28.x-dev
dashboard                  1.28.x-dev
(...)

Enabled contrib modules            
-----------------------            
backup_migrate             1.x-1.0.26
entity_plus                1.x-1.0.21
entity_token               1.x-2.0.4
entity_ui                  1.x-1.0.2
node_convert               1.x-1.2.0-beta2
rules                      1.x-2.14.1
(...)

quicksketch commented 7 months ago

we're only displaying enabled modules. Should that be mentioned?

I actually had that first, but when scanning the list, I found that I wasn't reading the main word "Core", "Contrib", etc in each heading when "Enabled" was in front of it (I'm a super-lazy reader). I also tried Core modules (enabled), Contrib modules (enabled), etc, but also found that to be weirdly phrased when there weren't any disabled modules listed. I might be naive here but if someone has requested a debug report, they'll know that it only lists enabled modules.

olafgrabienski commented 7 months ago

Yeah, I have exactly the same feelings about the alternatives: not easy to scan vs. looking awkward :-/

Maybe there's another way to make things clear? Otherwise people might expect to only see enabled modules but they can't be sure, and this might be puzzling in a debug situation. So, they'd have to ask, or to compare the debug list with the modules page. Hm, I really don't like that.

argiepiano commented 7 months ago

@quicksketch, @klonos it's unclear if my error above was fixed. Can you let me know? I'll try to test again later

argiepiano commented 7 months ago

OK, I just tested. The warning has not been fixed. Again, this happens because my active theme is a custom subtheme without a version number. This is not an uncommon occurrence. Settings this back to NW once again.

bugfolder commented 7 months ago

Yeah, I have exactly the same feelings about the alternatives: not easy to scan vs. looking awkward :-/

Maybe there's another way to make things clear?

We certainly don't need to list the word "Enabled" many times. How about a "super-heading" of "Enabled modules:", and then each section heading can just be "Core", "Contrib", etc.

quicksketch commented 7 months ago

I'll give that a go @bugfolder.

Also another small issue, the "Return Right Arrow" (⮑) displays as a missing character on my computer, seemingly regardless of the font used. The similar but slightly different "Downwards Arrow With Tip Rightwards" (↳) displays fine. I'll switch that out unless there's a better recommendation. This is what Return Right Arrow looks like to me:

quicksketch commented 7 months ago

I updated the PR at https://github.com/backdrop/backdrop/pull/4446:

Fixed the display of the base theme arrow
Fixed PHP notices when version strings were missing on themes and modules
Added "Enabled modules" as a header above all the module sections.


Backdrop CMS:              1.28.x-dev
Installation profile:      standard
PHP version:               8.3.2-1+0~20240120.16+debian11~1.gbpb43448
Database server:           10.0.38-MariaDB-1~xenial
Web server:                Apache/2.4.56 (Debian)
jQuery version:            3.7.1
jQuery UI version:         1.13.2

CKEditor 5, 40.2.0         Used in: Basic

Default theme:             My Theme 2 - Monochrome (my_theme2) 
   ↳ base theme:            ↳ My Theme - Monochrome (my_theme) 
     ↳ base theme:            ↳ Monochrome (monochrome) 1.x-1.0.3
Admin theme:               Seven* (seven) 1.28.x-dev
                           *used when editing or creating content

Enabled modules            
===============            

Core                       
----                       
admin_bar                  1.28.x-dev
block                      1.28.x-dev
ckeditor5                  1.28.x-dev
etc...

Contrib                    
-------                    
basic_cart                 1.x-1.1.1

Other                      
-----                      
devel                      1.x-dev
views_random_seed          1.x-dev

olafgrabienski commented 7 months ago

Looks wonderful!

stpaultim commented 7 months ago

This is so much better than what I asked for in the initial feature request. Thanks everyone!

I've added this feature description for the blog post (see initial post), feedback welcome.

"Backdrop CMS now includes a copy and paste debug report that can be used when reporting errors or seeking support in forums or chat. The new debug report includes server info, database version, jQuery info, a list of enabled modules and themes, and more. We expect that this report will make it much easier for site owners to get answers to technical questions, by making it easier for them to share the specifics of their site installation."

izmeez commented 7 months ago

Very nice. How about CKEditor 5, 40.2.0 Used in formats: Basic?

quicksketch commented 7 months ago

Thanks @izmeez for reviewing! I made two more changes:

Added the word "formats" to "Used in formats:"
Trimmed the excess white-space from the end of lines.

klonos commented 7 months ago

I've only been away for less than a couple of days, and look at you guys! 😄 ❤️

@klonos Do you think it's possible to put a label or a heading at top of the modules list?

I updated https://github.com/backdrop/backdrop/pull/4446 with a few changes, I hope that's okay @klonos.

By all means @quicksketch 🙂 ...and that goes to everyone that wants to use any of my PRs either as a starting point to add/improve upon, or as inspiration for their own alternative PRs. Be my guest - that's why I'm putting the code out there for in the first place.

Separated modules by the categories: core, contrib, custom, and other.

Thanks for that 🙏🏼 ...that might be something that we want to abstract and reuse in issues like #3146. Something that becomes available information in the arrays returned by system_rebuild_module_data() and module_list() perhaps.

I switched [core] to be the actual core value. Yes it's a bit redundant, but IMO that's better than needing to scroll up to the first line to find the core version.

Good call 👍🏼 ...although that can be moved slightly up in the code, so that it can be used in the $system_info array a few lines up, where BACKDROP_VERSION is used again. I'll pull your changes and update the PR branch with follow-up commits.

...because I encountered a problem where we may want the same line label multiple times (such as a row made of dashes to form an underline)

I have some ideas re that, but can be follow-up tweaks - don't want to derail this much. What @stpaultim said:

We can make it better if we need to later, but this is already much better than what I originally envisioned!

General question: Should we be passing any of these strings through t()?

I think so, yes 👍🏼

I found that I wasn't reading the main word "Core", "Contrib", etc in each heading when "Enabled" was in front of it (I'm a super-lazy reader)

That's not just you @quicksketch - it's a general UX thing. The main item of focus should ideally be mentioned first in strings of things like form element labels, headings etc. Same applies here.

Also another small issue, the "Return Right Arrow" (⮑) displays as a missing character on my computer ...

Yeah, that's a unicode issue I believe. Good catch 👍🏼 ...although I really liked that unicode character better, I also understand that making sure that the arrow is shown in most cases is a priority.

Related (but can certainly be a follow-up improvement): a common things at work, when troubleshooting site problems and using drush pml is that in certain scenarios there are contrib modules that do not necessarily namespace their submodules with the machine name of the parent/container module as a prefix. Sometimes they even have a prefix which is the namespace of another module entirely. One such recent case was the link_attributes contrib module in Drupal. Its v2.x version incudes two submodules:

link_attributes_menu_link_content (properly prefixed/namespaced)
linkit_attributes (namespace had me fooled and went looking for this as a submodule in the linkti project ...then back to the link_attributes module repo ...then scratch my head because it wasn't there either ...then randomly check if other branches exist that might have it ...then finally had an aha! moment)

In such cases, it would be useful to also "nest" submodules under their parent (the way we are doing now with the base themes and their subthemes), which would save people from similar "module hunting" rabbit holes ...and perhaps also omit the version from the submodules(?) ...but follow-up as I said.

I'll work on it some more soon and try to bring it to the finish line.

quicksketch commented 7 months ago

@klonos Sorry I pushed yet another commit I had in progress before I saw your comment. I added a collection of file system paths to the report, as well as "Drupal compatibility":

Backdrop CMS:                1.28.x-dev
Installation profile:        standard
PHP version:                 8.3.2-1+0~20240120.16+debian11~1.gbpb43448
Drupal compatibility:        off
Database server:             10.0.38-MariaDB-1~xenial
Web server:                  Apache/2.4.56 (Debian)
jQuery version:              3.7.1
jQuery UI version:           1.13.2
CKEditor 5, 40.2.0           Used in formats: Basic

Settings.php path:           /var/www/html/settings.php
Files directory:             /var/www/html/files (writable)
Temp directory:              /tmp (writable)
Config active location:      /var/www/html/files/config_a1614e81308b9e6b86be181ef0914335/active
Config staging location      /var/www/html/files/config_a1614e81308b9e6b86be181ef0914335/staging

Themes
======
Default theme:               My Theme 2 - Monochrome (my_theme2)
   ↳ base theme:              ↳ My Theme - Monochrome (my_theme)
     ↳ base theme:              ↳ Monochrome (monochrome) 1.x-1.0.3
Admin theme:                 Seven* (seven) 1.28.x-dev
                             *used when editing or creating content

Enabled modules
===============

Core
----
admin_bar                    1.28.x-dev
block                        1.28.x-dev
etc...

Contrib
-------
basic_cart                   1.x-1.1.1

Other
-----
devel                        1.x-dev
views_random_seed            1.x-dev

I realize as we're doing this we're essentially building a duplicate of the Status Report page. Bee has something similar in status_bee_callback(), so now we've basically got the same sort of checking in 3 places. Though I admit the kind of information you want to display in each place is probably different, with the status report being least granular, then the debug report, then bee reporting the most granular and technical information.

But that said, I don't think we should at this point try to abstract this out. The report page already has a lot of value as it is already.

klonos commented 7 months ago

@klonos Sorry ...

That's a good one. ...apologizing for something good that you've done 😅 ...again, you are more than welcome, and thank you 🙏🏼

izmeez commented 7 months ago

Is revealing the full path name of the active config directory through the GUI a concern? Should it be truncated?

klonos commented 7 months ago

@izmeez only user accounts with appropriate permissions will have access to this report (the access site reports permission, which is the same as the one required to access the site status report). When people copy/paste that information somewhere, they are free to redact whatever they don't want to disclose. So no, I personally don't think that it is a security concern.

klonos commented 7 months ago

@quicksketch just saw the addition of the Drupal compatibility on the report too. Nice 👍🏼

izmeez commented 7 months ago

Is it helpful or even necessary to identify the Default download method as Public or Private local files, without providing the full path name?

izmeez commented 7 months ago

When I saw the Drupal compatibility I took a double take. Would it be better to say "D7 compatibility layer" off/on? After all, in general, Backdrop is a lot like drupal.

klonos commented 7 months ago

... I don't think we should at this point try to abstract this out.

6492 😉

izmeez commented 7 months ago

Actually, is the D7 compatibility module more appropriate under themes ?

quicksketch commented 7 months ago

Actually, is the D7 compatibility module more appropriate under themes ?

The D7 compatibility layer wraps hundreds of drupal_* functions in their Backdrop equivalents, and isn't theme-layer specific. You can see the scope of it in the core/includes/drupal.inc file. But I agree on your other point, we should probably display that as "D7 compatibility" not "Drupal compatibility" at this point.

klonos commented 7 months ago

...we should probably display that as "D7 compatibility" not "Drupal compatibility" at this point.

Agreed 👍🏼 ...good catch @izmeez 🙏🏼 ...I'll make sure that I fix that in the next commits - just thinking that full "Drupal 7" instead of the "D7" abbreviation.

quicksketch commented 7 months ago

I snuck in yet another commit:

Now says "Drupal 7 compatibility"
Added "Private directory" for private files, as well as its writable/configured state.
Fixed the tests that had failed after adding other items.

argiepiano commented 7 months ago

I love this (and thanks for fixing the issue I reported above). Question: I notice that my custom modules are only listed under the Custom heading if they are in a folder called custom - otherwise they are listed as Other. AFAIK it's not typical or required for custom modules to be placed in modules/custom, or is it? This seems like an arbitrary decision in this PR.

quicksketch commented 7 months ago

AFAIK it's not typical or required for custom modules to be placed in modules/custom, or is it? This seems like an arbitrary decision in this PR.

We always do it on our projects at Lullabot and many (or even most) projects I contract on also do this, even without our involvement. I think it was listed as a best-practices early in Drupal and has stuck around, even though it's not required. But in any case, there's no way to discern between a custom module and one that hasn't run through the backdropcms.org packager (such as contrib modules that use a dev version). I think that sites that use "/modules/custom/" might get a little benefit from splitting those out, but if they don't everything that's not been through the packager (or in /modules/contrib/) ends up under "Other".

klonos commented 7 months ago

I snuck in yet another commit

You beat me to it ...only in my local implementation I used file_get_stream_wrappers(STREAM_WRAPPERS_ALL) instead of manually compiling the list of directories. That has the advantage to be reporting on other non-core-provided paths, but the disadvantage to only be including the private files directory if configured. That's why in #6494 I mentioned this:

... The change would most likely need to happen in the system_stream_wrappers() hook implementation, which would be useful if it also allowed an optional parameter that would cause it to also return information about the private files stream wrapper even if no path has been configured for it.

Follow-ups, follow-ups, follow-ups 😄

klonos commented 7 months ago

...I have a thought with re to the previous comment + a minor code tweak. Bear with me please.

backdrop / backdrop-issues

Add a report with copy/paste debugging information. #5791

Description of the need

Proposed solution

Alternatives that have been considered

Additional information

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

6492 😉