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

CRM-19303 - Refactor getDefaultFileStorage() function #9445

Closed
wants to merge 1 commit into from

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Nov 24, 2016

@colemanw
Copy link
Member Author

This attempts to make the code readable by refactoring the "if the cms is this then do that" logic into separate functions, per the fixme left by @totten.

The only substantial change is to the drupal function, which no longer relies on guessing the sitename based on the location of the civicrm module, but instead takes it directly from the files directory.

I have not tested this with multisites in D7, and haven't tested at all on D8 or D6, but I think it should work. Some actual testing would be welcomed.

Easiest way to test is to type CRM.config.CKEditorCustomConfig in your console, and see if the object contains a correct url (paste the url into your browser and check it, also verify it has the correct sites/sitename/files sitename).

@seamuslee001
Copy link
Contributor

Looks good @colemanw

@colemanw
Copy link
Member Author

Actually it occurs to me that the code I wrote for Drupal would probably work for every CMS. Maybe that's the only function we need.

@colemanw
Copy link
Member Author

Looking into it more, I think the code that was written for Wordpress is the best, and ought to work on every CMS.
So here is a more aggressive PR: #9458.
It will need testing in Drupal and Joomla!

@colemanw colemanw closed this Nov 28, 2016
@colemanw colemanw deleted the CRM-19303 branch August 7, 2019 03:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants