dev/core#2490 Drupal8: Fix CMS intergration table mapping

[olivierh65]

In Drupal8, CMS intergration don't show table mapping, because $database global variable is no more defined and can be retreived using Drupal\Core\Database\Database::getConnectionInfo()

Before

On the "CMS database integration" page, when running CiviCRM/Drupal on separate databases, the CMS database settings are not visible:

After

Technical Details

Fetches the Drupal8 database info from the place where D8/D9 now manages this.

Comments

edits by mlutfy.

[civicrm-builder]

Can one of the admins verify this patch?

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[demeritcowboy]

x-ref: https://lab.civicrm.org/dev/core/-/issues/2490

[eileenmcnaughton]

add to whitelis

[eileenmcnaughton]

add to whitelist

[eileenmcnaughton]

We probably should put the funciton on the drupal classes & then it becomes

$databases = CRM_Core_Config::singleton()->userSystem->getDatabases();

with the functions being called being on CRM_Utils_System_Drupal etc

[demeritcowboy]

Agree just I'm also trying to figure out why this patch isn't working for me locally. Regardless of the location it seems like it should.

[demeritcowboy]

Oh, it would help if I update the right install. So yes it works.

Actually this whole code function is full of if cms stuff. Maybe could leave that cleanup for a followup?

[demeritcowboy]

@olivierh65 The test run is failing because there are blank spaces on lines 46 and 54 and the style checker doesn't like that.

[demeritcowboy]

• General standards
  • [r-explain] PASS
  • [r-user] PASS
  • [r-doc] PASS
  • [r-run] PASS
    • Works in drupal 8/9 and 7.
• Developer standards
  • [r-tech] Soft Pass
    • As noted earlier it would be better to refactor to use OO-style and the CRM_Utils_System hierarchy, however the whole block of surrounding code is full of if [cms] for wordpress and backdrop too and that needs improving too. And why even bother with the $databases wrangling unless you're already inside the if $config->userSystem->viewsExists(). So there are a couple things for future if someone wants to take them on.
  • [r-code] Soft Pass
    • Usually commits are squashed before merging.
    • It might be safer to have the leading \ on Drupal\Core\Database\Database, but as noted above the whole thing should eventually be moved out.
  • [r-maint] ?
    • Difficult to write a test for.
  • [r-test] PASS

[totten]

Agree with the 'Soft pass' comments under r-tech/r-code -- the patch is consistent with the context/style around it. Yes, the existing page is wonky. But this page specifically calls-out things that haven't been abstracted well across CMSs, so I think it's gonna be a little wonky regardless. For fixing this concrete problem... the approach in the patch is sensible. So kudos @olivierh65.

But IMHO the r-explain needs work -- i.e. it's still got a large blob of template/boilerplate text. If the Gitlab issue summarizes everything you need to know, then that's cool - but the description should have the link and should clean-up the boilerplate.

[mlutfy]

I took the liberty of editing the PR summary to add more details.

[totten]

Looks good. Thanks @mlutfy @demeritcowboy @olivierh65