Migrate CiviGrant component to an extension #22064

colemanw · 2021-11-12T20:38:22Z

Overview

This a straight port of CiviGrant to be packaged as an Extension instead of a Component. All the functionality is the same aside from how it's enabled or disabled in the UI.

Before

Enable or disable CiviGrant through Administer -> System Settings -> Components. As a component it cannot be uninstalled.

After

Enable, disable or uninstall CiviGrant through Administer -> System Settings -> Extensions. As an extension it can be uninstalled, which completely removes grant data from the database.

For existing sites, the extension will be automatically installed as either "enabled" or "disabled" depending on the status of the component.

For new sites, the extension is not installed by default.

civibot · 2021-11-12T20:38:24Z

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

seamuslee001 · 2021-11-16T03:49:38Z

mysql: [Warning] Using a password on the command line interface can be insecure.
ERROR 1146 (42S02) at line 3795: Table 'core220642fa2jcivi_z4j89.civicrm_grant' doesn't exist

JoeMurray · 2021-11-22T11:19:26Z

@monishdeb could you do a review of this. Please include how this impacts extensions like Grant Application Pages, Grant Programs. I think there may be a profile edit one, and at least one financial regarding Sage Intacct. What is of interest and unlikely to be found is breakage of related extensions when CiviGrant is changed from Component to Extension. I'd focus on calls and installs. If related extensions need to check for enabled status of CiviGrant as an extension now that is fine.

monishdeb · 2021-11-25T12:12:10Z

@colemanw when I tried to install the civigrant extension on a clean setup I got this error:

Nov 25 12:06:33  [info] $ManagedEntities_failed = Array
(
    [entity] => OptionValue
    [action] => create
    [params] => Array
        (
            [version] => 4
            [values] => Array
                (
                    [option_group_id.name] => grant_status
                    [label] => Submitted
                    [value] => 1
                    [name] => Submitted
                    [grouping] => 
                    [filter] => 0
                    [is_default] => 1
                    [weight] => 1
                    [description] => 
                    [is_optgroup] => 
                    [is_reserved] => 
                    [is_active] => 1
                    [icon] => 
                    [color] => 
                    [component_id] => 
                    [domain_id] => 
                    [visibility_id] => 
                )

            [checkPermissions] => 
        )

    [result] => Array
        (
            [fields] => Array
                (
                    [0] => option_group_id
                )

            [error_code] => mandatory_missing
            [entity] => OptionValue
            [action] => create
            [is_error] => 1
            [error_message] => Mandatory values missing from Api4 OptionValue::create: option_group_id
        )

)

Have you encountered this strange error earlier?

colemanw · 2021-11-27T02:26:14Z

@monishdeb I just tried it myself and was able to run it without error. I ran setup to generate a fresh database based on this branch to avoid conflicts:

git checkout civigrant
./bin/setup.sh
cv en civigrant

I also just made some additional fixes and force-pushed this branch. 3 things are still not working as they should:

Advanced search / Find grants
The contact summary tab (which is based on the same search code)
Reports

monishdeb · 2021-12-08T12:20:25Z

ext/civigrant/managed/OptionGroup_advanced_search_options_OptionValue_CiviGrant.mgd.php

+    'update' => 'unmodified',
+    'params' => [
+      'version' => 4,
+      'values' => [


@colemanw shouldn't the parameters declared directly under params not under values ?

@monishdeb - no because APIv4 structures it this way. civicrm_api4('OptionValue', 'create', ['values' => [...]])

monishdeb · 2021-12-08T12:43:06Z

@colemanw I have rebased the PR. However I have one concern on the mgd file. Can you please take a look ?

colemanw · 2021-12-08T13:53:15Z

@monishdeb we'll have to rebase this again and move the upgrade code to 5.46. That'll be easier after merging #22224

monishdeb · 2021-12-08T14:50:04Z

I have reviewed and marked #22224 with MoP. Reviewed the current PR in my local setup, everything works fine. Tested by submitting new Grant from and didn't encounter any issue

colemanw · 2021-12-08T15:31:09Z

@monishdeb did you test all of these?

Advanced Search
Grants tab on contact summary screen
Grant reports

colemanw · 2021-12-09T16:57:44Z

@monishdeb I've got this rebased again to use the 5.46 upgrader. But I think search/reports still need testing per my above comment.

monishdeb · 2021-12-09T20:16:35Z

@colemanw yep my bad ... I missed the advanced search, reports . Will update the PR shortly with search fixes.

colemanw · 2021-12-09T20:25:50Z

Sounds good @monishdeb - FYI you'll have to reset your local branch because of the rebasing I just did.

seamuslee001 · 2022-01-04T02:58:33Z

ext/civigrant/CRM/Grant/DAO/Grant.php

          'import' => TRUE,
          'where' => 'civicrm_grant.application_received_date',
          'export' => TRUE,
          'table_name' => 'civicrm_grant',
          'entity' => 'Grant',
-          'bao' => 'CRM_Grant_BAO_Grant',
+          'bao' => 'CRM_Grant_DAO_Grant',


@colemanw any reason why this is being changed from the BAO to DAO?

🤷 GenCode wanted to...

Insert and then delete CiviGrant to avoid messing up legacy code with expectations of component IDs.

…table in core Removes CiviGrant from hardcoded lists when possible, or at least puts up guards to check that it's available before attempting to use it.

This fixes up the `CRM_Contact_BAO_Query_Interface` class which had incorrectly omitted `static` from some static functions. Adds defaultReurnProperties to search hook interface.

Disable & delete component, install extension as either "enabled" or "disabled" depending on the status of the component. Migrate option values to managed entities Migrate navigation menu to extension managed entities

colemanw · 2022-01-10T21:33:28Z

retest this please

eileenmcnaughton · 2022-01-11T02:07:34Z

I chatted with @colemanw and as

we are early in the release cycle and
this PR has gotten stale a lot and
a lot of work has gone into it

I'm merging after a light 'click around' and we should continue to QA during the release cycle.

@colemanw told me there is still a report issue - although I didn't spot it when I ran the report just now (as demo user). However I did spot a couple of things

permissin incongruities - as demo user I could enable the extension but obvoulsy have a permission deficit as I could not

click on the contact grants tab without error
see civigrant menu items such as add & search
But I COULD
see the grants tab that I couldn't click on
search for grants in advanced search
access civi reports

demo site set up - once the above is sorted we should ensure civigrant is enabled & demo user has access on demo sites

FYI @seamuslee001 @demeritcowboy

MegaphoneJon · 2022-03-11T22:03:02Z

The changes to CRM_Contact_BAO_Query_Interface - adding defaultReturnProperties() and setting from(), select(), and where() as static - breaks any Civi install with an extension that implements hook_civicrm_queryObjects. I'm not sure there's anything to be done now except to submit PRs to extensions though.

eileenmcnaughton · 2022-03-13T05:14:51Z

@MegaphoneJon I would be OK reverting - but I worry that extensions may have already adapted so that could break them....

Note it is implementing CRM_Contact_BAO_Query_Interface not the hook that is the issue (although probably both are done together). If I were maintaining an extension I'd probaly remove the extends though so it works on before & after

civibot bot added the master label Nov 12, 2021

colemanw force-pushed the civigrant branch 2 times, most recently from 001cd4b to 0fd1ed2 Compare November 16, 2021 03:07

colemanw force-pushed the civigrant branch from 0fd1ed2 to 524528d Compare November 18, 2021 00:36

colemanw mentioned this pull request Nov 18, 2021

OptionValue - Fix checking if domain is set #22095

Merged

colemanw force-pushed the civigrant branch 6 times, most recently from 35b7192 to 8727067 Compare November 19, 2021 12:14

JoeMurray assigned JoeMurray and monishdeb Nov 22, 2021

colemanw force-pushed the civigrant branch from 8727067 to 0277c2d Compare November 27, 2021 02:22

colemanw force-pushed the civigrant branch 5 times, most recently from 17b9c8e to d02b9b7 Compare November 29, 2021 19:04

monishdeb reviewed Dec 8, 2021

View reviewed changes

monishdeb force-pushed the civigrant branch from d02b9b7 to 79412ae Compare December 8, 2021 12:42

colemanw force-pushed the civigrant branch from 79412ae to 24262bc Compare December 9, 2021 16:56

colemanw force-pushed the civigrant branch 2 times, most recently from 04d8dd3 to 1b276ef Compare January 3, 2022 01:20

colemanw mentioned this pull request Jan 4, 2022

Fix campaign dashboard to work with default escaping #22364

Merged

seamuslee001 reviewed Jan 4, 2022

View reviewed changes

colemanw added 10 commits January 10, 2022 12:24

Generate CiviGrant extension

3406690

CiviGrant - Move CRM & API code from core to extension

0b9002f

CiviGrant - Migrate component functions to hooks

be49cd8

CiviGrant - Migrate schema files

678ca6f

Insert and then delete CiviGrant to avoid messing up legacy code with expectations of component IDs.

CiviGrant - Move report files

1d2d2e6

CiviGrant - Move unit tests

06eef90

CiviGrant - Remove or mitigate hardcoded references to civicrm_grant …

3c609ef

…table in core Removes CiviGrant from hardcoded lists when possible, or at least puts up guards to check that it's available before attempting to use it.

CiviGrant - Migrate legacy search query code

adde8f5

This fixes up the `CRM_Contact_BAO_Query_Interface` class which had incorrectly omitted `static` from some static functions. Adds defaultReurnProperties to search hook interface.

CiviGrant - Install extension + managed entities during upgrade

df01482

Disable & delete component, install extension as either "enabled" or "disabled" depending on the status of the component. Migrate option values to managed entities Migrate navigation menu to extension managed entities

CiviGrant - Regenerate SQL

29d2ce6

colemanw force-pushed the civigrant branch 2 times, most recently from 9159fc0 to 29d2ce6 Compare January 10, 2022 19:55

colemanw marked this pull request as ready for review January 10, 2022 19:55

eileenmcnaughton merged commit e50b42b into civicrm:master Jan 11, 2022

eileenmcnaughton deleted the civigrant branch January 11, 2022 02:09

colemanw mentioned this pull request Jan 11, 2022

CiviGrant - Use SearchKit for contact summary tab #22467

Merged

MegaphoneJon mentioned this pull request Mar 11, 2022

add defaultReturnProperties to not break in 5.47+ JMAConsulting/biz.jmaconsulting.activitytypeacl#23

Closed

colemanw mentioned this pull request Mar 13, 2022

Fix backward-compatability for new method in CRM_Contact_BAO_Query_Hook #22933

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate CiviGrant component to an extension #22064

Migrate CiviGrant component to an extension #22064

colemanw commented Nov 12, 2021 •

edited

Loading

civibot bot commented Nov 12, 2021

seamuslee001 commented Nov 16, 2021

JoeMurray commented Nov 22, 2021

monishdeb commented Nov 25, 2021 •

edited

Loading

colemanw commented Nov 27, 2021

monishdeb Dec 8, 2021 •

edited

Loading

colemanw Dec 8, 2021 •

edited

Loading

monishdeb commented Dec 8, 2021

colemanw commented Dec 8, 2021

monishdeb commented Dec 8, 2021

colemanw commented Dec 8, 2021

colemanw commented Dec 9, 2021

monishdeb commented Dec 9, 2021

colemanw commented Dec 9, 2021

seamuslee001 Jan 4, 2022

colemanw Jan 4, 2022

colemanw commented Jan 10, 2022

eileenmcnaughton commented Jan 11, 2022

MegaphoneJon commented Mar 11, 2022 •

edited

Loading

eileenmcnaughton commented Mar 13, 2022

Migrate CiviGrant component to an extension #22064

Migrate CiviGrant component to an extension #22064

Conversation

colemanw commented Nov 12, 2021 • edited Loading

Overview

Before

After

civibot bot commented Nov 12, 2021

seamuslee001 commented Nov 16, 2021

JoeMurray commented Nov 22, 2021

monishdeb commented Nov 25, 2021 • edited Loading

colemanw commented Nov 27, 2021

monishdeb Dec 8, 2021 • edited Loading

Choose a reason for hiding this comment

colemanw Dec 8, 2021 • edited Loading

Choose a reason for hiding this comment

monishdeb commented Dec 8, 2021

colemanw commented Dec 8, 2021

monishdeb commented Dec 8, 2021

colemanw commented Dec 8, 2021

colemanw commented Dec 9, 2021

monishdeb commented Dec 9, 2021

colemanw commented Dec 9, 2021

seamuslee001 Jan 4, 2022

Choose a reason for hiding this comment

colemanw Jan 4, 2022

Choose a reason for hiding this comment

colemanw commented Jan 10, 2022

eileenmcnaughton commented Jan 11, 2022

MegaphoneJon commented Mar 11, 2022 • edited Loading

eileenmcnaughton commented Mar 13, 2022

colemanw commented Nov 12, 2021 •

edited

Loading

monishdeb commented Nov 25, 2021 •

edited

Loading

monishdeb Dec 8, 2021 •

edited

Loading

colemanw Dec 8, 2021 •

edited

Loading

MegaphoneJon commented Mar 11, 2022 •

edited

Loading