Add missing is_public flags to public paths

[JKingsnorth]

Overview

The menu definition has a flag for 'is_public' that defines if a path can be viewed by 'the public'. Sometimes this includes 'the public, but with the right permissions (eg: 'make online contribution' pages are still 'public').

This PR adds the is_public flag to all pages that have an access callback of '1', but are missing the 'is_public' flag.

The exception is the 'civicrm/admin/scheduleReminders' path. Although it has an access callback of 1 it is not a public page, and some weird logic has been implemented to make it visible in the correct circumstances (see https://github.com/civicrm/civicrm-core/pull/6126/files)

An additional fix included is a very old typo (by the looks of it) next to the 'scheduled reminders' title field.

Before

Public paths were missing the is_public flag

After

More paths are flagged as is_public

Technical Details

Changes to XML menu definitions that will be automatically rebuilt into the DB on a cache rebuild.

[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).

[eileenmcnaughton]

@JKingsnorth what are the practical impacts of this? I'm guessing you hit some weirdness on one of them & decided to cleanup across all of them?

[seamuslee001]

@eileenmcnaughton isn’t it the is_public key what defines if something is front end or not in Drupal for the theme subsystem?

[eileenmcnaughton]

@seamuslee001 I would think so - but these seem like strange paths to have encountered theming concern on

[JKingsnorth]

I think the only functional uses are in CRM_Core_Invoke. It is used to set the breadcrumb, perform the 'statusCheck' appropraitely and set the 'urlIsPublic' template variable which is mentioned in 4 templates.

But this is more for consistency in case things people develop are relying on is_public being set consistently across all pages/paths that should be accessible to public users.

We had something in mind for this, which is what made me notice the inconsistency.

[seamuslee001]

This makes sense to me i think we should do this merging

[jensschuppe]

Can someone explain why CRM_Report_Page_InstanceList and CRM_Report_Page_Instance should be marked public, just because there is no generic access callback? This PR indeed causes all reports be displayed using the "Public" theme, which is different behavior compared to versions prior to 5.17.0. Honestly, I would call this a UI regression, since there is no way to restore the "old" behavior.

[JKingsnorth]

Hiah, yes there is no access callback, so the pages can be displayed to the public, so I gave them the is_public flag.

This causing the pages to render in the 'public' theme is an unintended consequence. So I can see the argument for removing the is_public flag from these paths, since they're not really 'public' in any use case I can think of. I can create a PR for this.

[jensschuppe]

I can create a PR for this.

That would be much appreciated, thanks.

I'm not totally convinced though that having no generic access callback is the same as being a public page - what actually denotes a CiviCRM page being "public" (compared to "not really public") apart from potentially being rendered in a different theme (asking out of curiosity)?

[JKingsnorth]

what actually denotes a CiviCRM page being "public" (compared to "not really public") apart from potentially being rendered in a different theme (asking out of curiosity)?

Nothing, just the 'is_public' flag, and the 'intended use' of the page. The aim of this PR was just to tighten up the use of the flag, so that it can be more useful.

The is_public flag sets the theme, and a variable on the page. But it is useful to have. We use the is_public flag so that we can block all access to 'non-public' pages from outside our administration network, for example.

[JKingsnorth]

See #15261