Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add resetPaths as per issue #2458 #1 #19795

Merged
merged 2 commits into from
Mar 14, 2021

Conversation

homotechsual
Copy link
Contributor

@homotechsual homotechsual commented Mar 12, 2021

Overview

This was raised as issue 2458 when deploying sites programatically through CI or CD pipelines it's often useful to be able to reset paths quickly and easily. This PR adds an APIv4 endpoint that can perform this task as System.resetPaths

Before

No way to reset paths using the API.

After

APIv4 endpoint to resetPaths in a similar way to the API endpoint for cache clearing system.Flush. Uses existing core functions to perform the "heavy lifting".

Technical Details

Essentially an APIv4 wrapper around CRM_Core_Bao_ConfigSetting::doSiteMove() relying on the existing core method entirely.

@civibot
Copy link

civibot bot commented Mar 12, 2021

(Standard links)

@civibot civibot bot added the master label Mar 12, 2021
@homotechsual
Copy link
Contributor Author

I'm leaving this WIP at the moment, it's complete and tested but I'm not sure it's the right approach, I'd like to hear input on whether there's a better way to do this!

Comment on lines 16 to 21
* Clear CiviCRM caches, and optionally rebuild triggers and reset sessions.
*
* @method bool getTriggers
* @method $this setTriggers(bool $triggers)
* @method bool getSession
* @method $this setSession(bool $session)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docblock looks incorrect. Copy/paste error?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep :-) will fix!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that this was copied from the System::flush action, and that makes me wonder if this even needs to be a standalone action or if we can just add one more param to that API, along with triggers and session we could add paths.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that this was copied from the System::flush action, and that makes me wonder if this even needs to be a standalone action or if we can just add one more param to that API, along with triggers and session we could add paths.

That sounds interesting, the reason I made it standalone was following the UI convention where flush and reset paths were separate. Open to reworking this so it's part of flush if that fits better :-)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No I think you're right. This is good as-is.

@colemanw
Copy link
Member

Go ahead and finish the PR description, I'm giving this merge-on-pass.

@colemanw colemanw changed the title [WIP] Add resetPaths as per issue #2458 #1 Add resetPaths as per issue #2458 #1 Mar 13, 2021
@eileenmcnaughton eileenmcnaughton merged commit da28128 into civicrm:master Mar 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants