dev/core#2000 Move afform into core extensions #18423
Conversation
|
(Standard links)
|
|
I re-ran civix & added a commit & also here https://lab.civicrm.org/extensions/afform/-/merge_requests/28/diffs |
eileenmcnaughton
force-pushed
the
afform
branch
from
c1b3958 to
df74c47
Compare
|
When we merge, I'd like to preserve history if we can, eg http://gbayer.com/development/moving-files-from-one-git-repository-to-another-preserving-history/ |
|
@totten hmm OK - did we do that for any of the others? |
|
Well, we haven't exactly done this a lot. :) For comparison:
Basically, if we have a situation where it is feasible to keep the history, then I think that's better. (I'm experimenting here: https://gist.github.com/totten/5881476fdea542a57d60815da37ea3cf ) |
|
@eileenmcnaughton I've updated the migration script at https://gist.github.com/totten/5881476fdea542a57d60815da37ea3cf -- it gives a branch in which:
Example output: If the idea is to continue patching |
|
@totten @seamuslee001 not sure about the last 2 style warnings |
|
@eileenmcnaughton add in lines like https://github.com/civicrm/civicrm-core/blob/master/ext/flexmailer/tests/phpunit/bootstrap.php#L14 around the eval |
eileenmcnaughton
force-pushed
the
afform
branch
from
ccca036 to
2e77c07
Compare
|
@totten with this merged to afform https://lab.civicrm.org/extensions/afform/-/merge_requests/30 style is solved - I think the next step is to close this & for you to do a replacement? |
|
@totten I see you just merged the PR into afform - what is the next step? Should I close this & you can do some magic? |
This allows you to invoke APIv4 when a user clicks on a button or link. It is analogous to `ng-click`, except that the notation and messaging are tweaked for APIv4.
Arguably afform still works on older versions but breaking changes have happened & if people use it on older you want a buyer-beware type feeling
This means that data can be set in an embedded form e.g
<af-entity data='{name_a : options.name_a, name_b : options.name_b, is_name_b_nickname: options.is_name_b_nickname, is_name_b_inferior: options.is_name_b_inferior}' .... />
In this case the options.name_as comes from the directive that instantiates this afform
The afField.getOptions function was causing infinite recursion by recreating the boolean options array every time. Passing this variable by refernce solves the problem, and adding "track by" to the ng-repeat follows best-practices for efficiency.
Civi 5.23 changes boolean options to be real true and false instead of '1' and '0' so added version bump to keep in step.
This reverts commit d31206f9e08703ae4f11b7e34673d7ece92b1630.
I removed the commented out functions rather than figuring out how to style them right
|
@eileenmcnaughton I've re-run the migration script and force-pushed to your branch. So this PR now has a full history. |
|
@seamuslee @Coleman afform PR is now passing. It was in last week's dev digest so we have communicated about it- but I think one of you should merge as it's technically my PR but all the commits were added by @totten |
|
Merged. I also archived the gitlab repo to avoid confusion. |
|
Cool - so test wiring just needs doing |
|
@totten can you pls enable the afform tests on core CI? |
Overview
Moving afform into core means we can start to recreate core forms as afforms, in particular where
they are problematic and / or hard to maintain.
Before
Maintained as separate repo
After
Maintained as part of core repo
Technical Details
Per https://lab.civicrm.org/dev/core/-/issues/2000 my understanding of the scope of this round of work is just to move it into core & wire it up. Future rounds of work will transition it from 'present' to installed - probably with installed happening first on buildkit installs. There are some technical issues for later rounds around the scenario where the extension is present in the core directory and in the extensions directory
Comments
We need to continue to have a lighter review requirement for afform while it's still not enabled on install