-
Notifications
You must be signed in to change notification settings - Fork 11
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
Fix alter hook name and placement #190
Conversation
Code Climate has analyzed commit 11f1bd8 and detected 0 issues on this pull request. View more on Code Climate. |
@@ -168,9 +168,9 @@ public function justDoIt() { | |||
|
|||
if (!empty($data)) { | |||
foreach ($data as $type => $results) { | |||
drupal_alter('capx_preprocess_results', $results, $this); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix on the typo.
Moving this line seems reasonable and the API documentation seems to support it but this is a change to how the alter function is expected to be used. In the previous position this alter only had a scope of one result item and would not have the ability to compare items against others in the response. What kind of preprocessing are you doing if you don't mind me asking?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sherakama I had a user request to make sure that "graduate" students were not imported (at all) into our system. So I am using this hook as the first exposure to the results gathered, searching through the results for contacts that contain a position of "graduate" and if they do have that position I am unseting the result so it never makes it to our end. Then in the other PR I put up, I apply the same code to remove these results from the orphan results too.
So knowing this, the root desire I get asked for routinely is to add in a feature that would exclude results. Specifics and workarounds listed below:
sunetid: (individuals) has been requested in the past, work around is to unpublish specific users and disable their automatic syncing from CAPx.
field values: (limited number of users) current request described above
suborg: (lots of users, lots of effort) this would be a big one. The reason that suborg would be a big one is because we currently have many importers because the parent orgs pull in too many users (work around is to pull in all the suborgs that we specifically want).
Thanks for the quick response and all the hard work you have put in here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the explanation @ccjjmartin,
I've got a couple of things that might make your life easier. If you haven't already, I would employ to you explore the https://github.com/SU-SWS/stanford_capx/tree/7.x-3.x/modules/capx_filters module. This module provides an interface for creating filters to prevent saving profiles to your site. You can combine this module with the https://github.com/SU-SWS/stanford_capx/tree/7.x-3.x/modules/capx_tampers module which has feeds like tampering functionality to narrow your imported result set down quite a bit. More help on those module can be found in our user guide: https://sites-jumpstart.stanford.edu/jumpstart-user-guide/stanford-content/people/filtering-imported-profiles-capx
The too many users issue is a real problem as we have quickly overrun available resources on both the API side and the client side when pulling in orgs with too many users. Until about 2 weeks ago this was the way life was. Recently the API team has release new functionality that will help with this as they have provided a way to filter out results on their end as well as provide a way to allow us to request only parts of the response data for each profile. Work on our end hasn't yet started. In the mean time, you can lower the number of profiles processed per operation by adjusting the stanford_capx_batch_limit
variable.
@sherakama Thanks for the suggestions. I will run them up the chain here and see if we can implement them. Appreciate the guidance. |
Purpose:
Testing:
Notes: