search

GSBCfamily / bvcms

The open source church management system
http://touchpointsoftware.com
GNU General Public License v2.0
1 stars 1 forks source link

Clear Button and SG Filter Options for Org Member Management #90

Open hkouns opened 8 years ago

hkouns commented 8 years ago

Karen has the following suggestions/questions as extensions to #7 :

nehresma commented 8 years ago

Heath do you mind if I pull your changes in the org_member_management_changes_mods branch (ffd83099fe4815476b0a9f9c1d8f3e57008254fe and 2e443ef292d10ffb257d3260782860d57d9187c0) into a new branch taken from bvcms/master? I'll add the None option and then we can have another clean PR for David.

hkouns commented 8 years ago

That would be great!

On May 31, 2016, at 7:25 PM, nehresma notifications@github.com wrote:

Heath do you mind if I pull your changes in the org_member_management_changes_mods branch (ffd8309 and 2e443ef) into a new branch taken from bvcms/master? I'll add the None option and then we can have another clean PR for David.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

nehresma commented 8 years ago

I've added the None support. You've done the other two on this issue. The branch is org_member_management_tweaks (very original name).

I've been testing it here locally and it seems to work fine. If it looks good to you, feel free to either create a PR to bvcms/master or let me know and I will.

hkouns commented 8 years ago

Nathan, In order to duplicate the function of the sub-group filter on the organization page, we would need to add an "Exclude" filter in addition to "Match All" and "None Assigned".

Do you think you could add that easily?

Thanks, Heath

Sent from my iPhone

On May 31, 2016, at 8:58 PM, nehresma notifications@github.com<mailto:notifications@github.com> wrote:

I've added the None support. You've done the other two on this issue. The branch is org_member_management_tweaks (very original name).

I've been testing it here locally and it seems to work fine. If it looks good to you, feel free to either create a PR to bvcms/master or let me know and I will.

� You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_GSBCfamily_bvcms_issues_90-23issuecomment-2D222864524&d=CwMCaQ&c=ByDrzdYw8tO08sJlHDO_Vg&r=iGjRQCxUMWIh2KzlFV0J1Q&m=6ALmpt1YmUIQqBxd92L2kceC8CcKu8b26CEyaP4GzGA&s=sjF3P0VWrF3YAcj_PUC1TJu8mFnTbew_zegetTsBPfg&e=, or mute the threadhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe_AE5xMdkaI9v8rsgKxWt5pafi0r6Dn6CAks5qHNkPgaJpZM4InmQY&d=CwMCaQ&c=ByDrzdYw8tO08sJlHDO_Vg&r=iGjRQCxUMWIh2KzlFV0J1Q&m=6ALmpt1YmUIQqBxd92L2kceC8CcKu8b26CEyaP4GzGA&s=zAhjka8wtFB0q9CEJE3voV_YxOJLxnENdeLuOn68QE4&e=.

nehresma commented 8 years ago

Ah, yes. Sorry, I misunderstood. Will do.

nehresma commented 8 years ago

@hkouns I've added support for leading dashes to exclude the sub-group. The searches can be combined with sub groups to include. For example: "Group 1;-Group 2" returns people in Group 1 and not in Group 2.

There is help text about it, but I did not include an "Exclude" option in the drop down. It wasn't clear to me how that would work particularly since there can be multiple Sub-Groups already populated in the text box. Further, when I went to look at how it works on the Organization page I noticed that it seems to be broken over there. Clicking it did absolutely nothing. So between not knowing what behavior to emulate from the Org page, and the semantics not being clear in the case where there are more than one group, I simply left it out for now. Leading dashes need to be entered manually.

If you'd like an "Exclude" it in the drop down, maybe we could talk in more detail about how it would work.

hkouns commented 8 years ago

@nehresma Try Clicking the Exclude option in the dropdown... and then click another sub-group name. Basically it adds the "-" in front of the Subgroup name.

Here is the documentation page describing the sub-group filter on the org page http://docs.touchpointsoftware.com/Organizations/FilterBySmallGroups.html#sub-group-filter

regarding the combined searches..... without "ALL" then Group1, Group2, Group 3 would perform an "OR" query... is it still an "OR" query when you add a "-" in front of one of the groups...or does it become an ALL? (we need to make it match the org page)

nehresma commented 8 years ago

Heath: Thanks for the pointers about the Exclude behavior. I've pushed changes so that when you click Exclude then click on a sub group name it inserts the "-" in front of the name.

As far as the logic goes, lets think about this as 2 different sets of operations: positive matches and negative (excluded) matches. All the positive matches are ORed together. So "Group 1;Group 2" is equivalent to "Member Tag = Group 1 or Member Tag = Group 2". All the negative (excluded) matches are AND-ed together. So "-Group 1;-Group 2" is equivalent to "Member Tag != Group 1 and Member Tag != Group 2".

Then the 2 sets themselves are AND-ed together. So "Group 1;Group 2;-Group 3;-Group 4" is roughly:

(Member tag = "Group 1" OR Member tag = "Group 2")
AND
(Member tag != "Group 3" AND Member tag != "Group 4")

I spent an hour digging through the code to try to find the expression builder logic for the Org page with no success. As far as I can tell from manually testing though, what we have matches the Org page's sub group filter logic. Copying/pasting sub group filters back and forth between the 2 pages and searching is giving me identical results on both pages.

Could you confirm when you get a chance?

hkouns commented 8 years ago

@nehresma thanks...I will try to test tomorrow.

I believe that the expression builder for the org page is here: https://github.com/bvcms/bvcms/blob/1a2f08fe333063e37bd3379a4a110fa654ee7fd6/CmsWeb/Areas/Org/Models/Org/OrgPeopleModel.cs#L56

...and this is what I said I didn't understand on Saturday at your house. I have not seen this construct before.

UPDATE: based on a comment David made on a Trello card. This query is constructed all in SQL.... and he made a change to this file in relation the the Org SG filter logic: https://github.com/bvcms/bvcms/blob/20531268db21f6439cfb53159fb1ba0eeeaddfec/SqlScripts/BuildDb/Creating_dbo_OrgPeople.sql

hkouns commented 8 years ago

@nehresma

Try looking at work site for the org: "Christmas Prison outreach"

When I use the filter: flexible;contact_caregivers;-contact_sponsors it gives me 3 people.

When I use the filter: All:flexible;contact_caregivers;-contact_sponsors it gives me 0 people.

Now go to the Org Management page (Program:outreach, Div:outreach, Org: ChristmasPrisonOutreach)

When I use the filter: flexible;contact_caregivers;-contact_sponsors it gives me the same 3 people.

When I use the filter: All:flexible;contact_caregivers;-contact_sponsors it gives me 1 person... Dee

Based on my understanding of the Logic. you have it correct and the Org page has a bug... do you agree?

hkouns commented 8 years ago

@nehresma 3rd comment of the night... oops... got carried away. ;-)

I made a couple minor changes to make the "None Assigned" match "None" wording of the Org page.

I loaded the new code to the work site and asked Karen to review. I also asked her to review the potential bug.

hkouns commented 8 years ago

@nehresma said

Thank you for the notes on GitHub and also the help testing it. Your results do show that the logic doesn't match between the two. I'm not quite sure how I was getting the same results between both pages.

So I looked a little bit this morning before work and I think that DefineModelList is the location we're looking for. Tracing it further, looks like it calls a SQL Server function where the subgroup logic is defined:

https://github.com/bvcms/bvcms/blob/master/SqlScripts/BuildDb/Creating_dbo_OrgPeople.sql

David's note here about writing the logic directly in SQL instead of letting Linq build the query also seems to confirm this.

We have a couple options here.

  1. I can try to make Linq generate queries with logic that is equivalent to what he has defined in SQL.
  2. We can refactor the way the Org Member Management queries are performed, not relying on Linq and instead write the query ourselves. It will most likely have a better execution plan in SQL Server than the ones Linq generates. Linq generates subqueries checking for match existence for each sub-group and that likely doesn't scale well (though I don't have a large enough database to do any real measurements to verify). Basically this would be doing what David's done on the Org page.

If there are churches with groups of 15k members, we probably want to go route #2 simply to make the page usable for them. It will also be easier to match the David's exact subgroup matching logic with #2. But it will take more time than #1 (schema changes, etc).

Thoughts?

I think that your current logic is the correct way... although I need to think about David's comment that: |group1,group2,-group3| would the equivalent of |(group1 OR group2) AND NOT |group3|

Do you agree with that? I would have thought initially that it would be: group1 OR group2 OR (NOT group3)

I agree that going with route #2 is probably better ... for the same reason that David did... and more-so because we are returning everyone from a division. So it could be pulling from multiple LARGE orgs.

There is no real timeline for this these features... so if it takes a little longer that is probably ok. I will mention this on the trello card and see what type of response we get.

hkouns commented 8 years ago

Actually, I just tested and group1, group2, -group3 is equivalent to (group1 OR group2) AND NOT group3

on both the org page and org management page on the test site.

hkouns commented 8 years ago

@nehresma David agreed on the Trello card we should move in that direction (#2)

His comment:

You should call the same SQL function that I use, I don't want to have that code duplicated anywhere.

Using the example you gave me, you would call it like so on your database:

SELECT PeopleId FROM dbo.OrgPeople(225, '10', NULL, NULL, 'All:flexible;contact_caregivers;-contact_sponsors', 0, NULL, NULL, 0, 0, 0, NULL)

nehresma commented 8 years ago

Hmm. Okay. We have additional filters so I'll need to wrap the call to OrgPerson in a new function that additionally filters with our stuff. Similar to how OrgPerson wraps OrgPerson2.

FWIW, this will probably take a bit of effort (read: time).

On 06/03/2016 02:52 PM, Heath Kouns wrote:

@nehresma https://github.com/nehresma David agreed on the Trello card we should move in that direction (#2 https://github.com/GSBCfamily/bvcms/issues/2)

His comment:

You should call the same SQL function that I use, I don't want to have that code
duplicated anywhere.

Using the example you gave me, you would call it like so on your database:

|SELECT PeopleId FROM dbo.OrgPeople(225, '10', NULL, NULL,
'All:flexible;contact_caregivers;-contact_sponsors', 0, NULL, NULL, 0, 0, 0, NULL)|

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/GSBCfamily/bvcms/issues/90#issuecomment-223662891, or mute the thread https://github.com/notifications/unsubscribe/AAN6qTUyozOsTEgbpVty9IBGSfxELjmFks5qIHgFgaJpZM4InmQY.

nehresma commented 8 years ago

Note to future self: populate dbo.Numbers before using the code in the OrgMembersModel2-Test branch.

nehresma commented 8 years ago

Just an update. I've read through the code in the OrgMembersModel2-Test branch and I've got it working locally. I need to go through and apply all our other changes in the org_member_management_tweaks branch to it yet. Its been a long day so I'm calling it a night a bit early. I'll start working on those other changes tomorrow.

hkouns commented 8 years ago

Thank you Nathan!

Sent from my iPhone

On Jun 7, 2016, at 8:24 PM, nehresma notifications@github.com<mailto:notifications@github.com> wrote:

Just an update. I've read through the code in the OrgMembersModel2-Test branch and I've got it working locally. I need to go through and apply all our other changes in the org_member_management_tweaks branch to it yet. Its been a long day so I'm calling it a night a bit early. I'll start working on those other changes tomorrow.

� You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_GSBCfamily_bvcms_issues_90-23issuecomment-2D224453380&d=CwMCaQ&c=ByDrzdYw8tO08sJlHDO_Vg&r=iGjRQCxUMWIh2KzlFV0J1Q&m=x7Or9wnqNr91hj7DB1gljFj423F47V3qwjSPfsxmsj0&s=slfPFHLdVTfQDoOfEZz2kxY6NJY90aiIM8OgqaTuQ&e=, or mute the threadhttps://urldefense.proofpoint.com/v2/url?u=https-3Agithub.com_notifications_unsubscribe_AE5xMWeTuXItRyyKw3REAaZYazLMJcHiks5qJguWgaJpZM4InmQY&d=CwMCaQ&c=ByDrzdYw8tO08sJlHDO_Vg&r=iGjRQCxUMWIh2KzlFV0J1Q&m=x7Or9wnqNr91hj7DB1gljFj423F47V3qwjSPfsxmsj0&s=g445MSgJYdipPZLNw62q-4S1WoLcTTBQi3VfUarot_4&e=.

nehresma commented 8 years ago

Heath, I've merged our changes into David's branch and pushed it into GSBCFamily/OrgMembersModel2-Test. Everything I've tested seems to work fine.

If David is happy with this, we can create a PR. For long term maintenance though, I would suggest that that we get rid of OrgMembersModel2 and merge its contents into OrgMembersModel. But of course that is Touchpoint's call as to how they want to manage the change.