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

dev/core#472 Allow for APIv3 to find examples in Extenion folders as … #13006

Merged
merged 1 commit into from
Oct 29, 2018

Conversation

seamuslee001
Copy link
Contributor

…well as core

Overview

This allows for the APIv3 Explorer to pick up examples that might be found in an extension in api/v3/examples/ just like how its in core

Before

Only Core examples found

After

Extension examples found as well as core

ping @eileenmcnaughton @colemanw @totten

@civibot
Copy link

civibot bot commented Oct 24, 2018

(Standard links)

@civibot civibot bot added the master label Oct 24, 2018
@eileenmcnaughton
Copy link
Contributor

I was able to get this to work by adding a folder/file to an extension called

examples/ReportTemplate/Getmetadata.php

This showed up in the examples. However, I note that you add the entity with checking if it is already present & without alphabetical sorting so I get

screenshot 2018-10-25 17 22 16

screenshot 2018-10-25 17 22 23

screenshot 2018-10-25 17 22 28

My other reservation are

  1. this adds more uncached file scanning (mitigated by the fact it only affects the api explorer page)
  2. The check doesn't exclude disabled extensions.
  3. unit tests would be good

SInce this is pretty edge functionality I could probably swallow those 3 if the duplicate entity were fixed

@eileenmcnaughton eileenmcnaughton added the sig:extension support Supports being able to integrate extensions with CiviCRM label Oct 25, 2018
@colemanw
Copy link
Member

I think the code here could be greatly simplified by scanning all directories in the php include_path instead of all this extension loading rigamarole. I've done it that way in a couple other places and it's worked out well, since Civi adds every extension directory to the include path.

This code for example looks in every spot in the include path for a crm-tutorials folder. You could do the same for an /api/v3/examples directory.

@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton I have now alpha sorted and switched to the include_path fucntion as suggested by @colemanw. By using the include_path then that solves 2. as only enabled extensions are on the included path. I don't think we can fully stop the duplicate entity thing because i don't believe there are API rules to stop that from happening.

// Remove this so that we only get one version of each folder.
if (substr($path, -7) == 'civicrm' || substr($path, -17) == 'civicrm//packages') {
continue;
}
Copy link
Member

Choose a reason for hiding this comment

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

@seamuslee001 I think this might be different on different sites, and the best way to dedupe would be a little preprocessing to remove trailing slashes before running array_unique (note that on some systems the slash will be a backslash)

@colemanw
Copy link
Member

I don't think we can fully stop the duplicate entity thing because i don't believe there are API rules to stop that from happening.

I think ideally the list would be deduped and all ReportTemplate examples would be shown for the combined list item.

@eileenmcnaughton
Copy link
Contributor

yep @seamuslee001 - the list double up issue can be fixed by array wrangling - e.g flip it & sort it & flip it back.

@seamuslee001
Copy link
Contributor Author

@colemanw @eileenmcnaughton i think the duplicate issue is fully dealt with now, I have also ensured my code is safeguarded for Windows and Non windows installs. I believe all concerns have now been addressed

…well as core

Improve inclusion scanning by using get_include_path as suggested by Coleamn and alpha sort the list of entities

Deal with windows directory separators and more helpfully handle the trailing and non trailing slash directories

Further fixes for windows Directory Separators
@colemanw
Copy link
Member

Unrelated test fail. Code looks good, and confirmed existing examples still work on the demo site.

@colemanw colemanw merged commit 11c954b into civicrm:master Oct 29, 2018
@eileenmcnaughton eileenmcnaughton deleted the lab_core_472 branch October 29, 2018 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
master sig:extension support Supports being able to integrate extensions with CiviCRM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants