GSBCfamily / bvcms

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

AutoPopulate Org Member Management Sub-Group #7

Closed hkouns closed 8 years ago

hkouns commented 9 years ago

Originally submitted as: Heath Kouns @ gsbcfamily [5700]

Feature Request - Org Member Management Sub-Group

I would like the sub-group field on the Org member Management page to accept a semi-colon separated list.

Use Case:

I have set up a registration page for VBS volunteers...

one question is for type of job they want (Crew leader, snacks, registration, etc)

another question is the age group they prefer (elementary age, nursery, etc.)

When I am moving crew leaders into the individual VBS crews, I would like to filter by two sub-groups (e.g.: CrewLeader, ElementaryAge)

Another longer term request... it would be nice if the sub-group field auto-populates a drop-down multi-selection box after you select a source org similar to the way it works on the Organization page.

hkouns commented 8 years ago

@nehresma

This is one of the next projects I would like to start working on... I would like to have in place before VBS this year.

There are several parts to this... some may or may not be included in the first release.

hkouns commented 8 years ago

This was originally requested due to inadvertent attempts to move people that were caused by the focus on the "Move" button.

hkouns commented 8 years ago

probably best done with a check box. Since you know the destination org.. you should be able to determine the leader type...then change the org member type of the person being moved... useful when moving VBS leaders into groups.

hkouns commented 8 years ago

Useful for copying people form a volunteer registration org into multiple Volunteer Calendar orgs. Maybe use a checbox for "Copy only" (change move button text to "Copy")

MOVE TO A NEW ISSUE - 05/25/16

nehresma commented 8 years ago

@hkouns, I count 8 different items on this issue. Do you mind if I break them up into 8 different issues and create PRs for each separately? I suspect it may be easier to get them merged upstream and for TouchPoint to test/QA them in smaller chunks than in a single PR.

hkouns commented 8 years ago

Yes, That's fine. The first 3~4 go together. If you do separate PRs... we will likely test all combined, then re-baseline and submit to TP.

hkouns commented 8 years ago

@nehresma check this commit for any impacts: https://github.com/bvcms/bvcms/commit/d800781acc734158cd8e76acb51897cd526b2c08 and this one: https://github.com/bvcms/bvcms/commit/591bbf25de70b63ac443914b2f649e5e5037cd1a

nehresma commented 8 years ago

@hkouns I've looked those over and neither of them should impact #82.

nehresma commented 8 years ago

Ok so I was wrong to try and do these in separate tickets since they build on top of each other. I wasn't thinking. So for the record 9281267b0405a1d57328cce7f1e305649fbdee43 is step 1 -- being able to filter by more than one Sub-Group.

Step 2 is in 2e55c1d -- auto populating a dropdown of sub groups for division and source. This is in the search_multiple_org_subgroups branch. I will attach a couple screenshots of the sub-group filtering in a moment.

I will work on refining grades by division and source and putting them in a drop down next.

nehresma commented 8 years ago

image

When I add the "Clear" button next to the drop down, things get too tight for space. So I went ahead and left it out for now since the user can just Control-A to highlight everything and press delete or backspace and accomplish the same thing.

nehresma commented 8 years ago

When no division is selected: image

When a division is selected but an Org that doesn't have any sub groups: image

When just selecting a division and no source is selected, all subgroups in the division are listed: image

nehresma commented 8 years ago

I've added the drop down for grades. @hkouns - a previous comment mentions filtering by age, but it includes a question mark. Is that desirable? We would need to figure out how to do that in a way that is usable since there could be a lot of unique ages.

Remaining items here (besides age if we want it and figure out a good interface for this) are the 3 enhancements.

nehresma commented 8 years ago

Screenshots of the grades dropdown:

image

image

image

hkouns commented 8 years ago

@nehresma thanks for your work on this... tested it out some tonight and things looked really good. Minor item to add text to Information button in Sub-Group control.

For age filtering... One thing to consider is that in typical use (e.g. moving kids between classes or for VBS, typically you would only be filtering for a few ages... so maybe the same format is ok Another option would be not to auto load the control... but instead make it a generic text box and then accept inputs like"<5", ">3" or "4-7" ... you would have to parse the textbox and decide on the type of filtering. (would also need to handle added spaces (e.g. "< 5")

of the enhancements... the most impactful is the 2nd ... changing a person's membership type to the to leader type of the target org on move ... (activated by toggling a checkbox) the first one is fairly trivial...but isn't a huge impact...just fixes an annoyance

the third is probably a more significant change and maybe should wait for a later PR.

Your thoughts?

Thanks Again!

nehresma commented 8 years ago

I fixed the popup help. It needed to be re-enabled after the form dynamically reloads. I'm thinking through the age filter now. It'll need to be a mini parser and predicate builder.

nehresma commented 8 years ago

@hkouns here's the age filter, along with range support. I spent several hours trying to figure out how to dynamically add predicates to an existing LINQ query and in the end I gave up just so I could get something done. So commit e9c8f5e certainly violates DRY, but it is working. :(

Here are a few screen shots of the Age filter in action.

image

image

image

image

image

nehresma commented 8 years ago

@hkouns here are a few screen shots of what the "change member type" functionality looks like. Does this envision what you were thinking? If so, I'll wire up the back end and push it. Then maybe we could see if TouchPoint will review it and possibly include it in a release prior to VBS.

General layout: image

When checked the member types are displayed: image

And the member types populated: image

hkouns commented 8 years ago

WOW....That is more than I was thinking...I was thinking to just change to the Leader of the Target Org... but I think this is more universal. Thanks

hkouns commented 8 years ago

I rebased the search_multiple_org_subgroups branch on the BVCMS master... when you add this code, could you add it to this branch... Hope it doesn't cause a problem.

At the risk of adding complexity... (And we can put it off if you think it is too much. Karen recently added a Trello card for her desire to add a TARGET DIV selector (which would make the existing DIV the SOURCE DIV selector) ... then the TARGET Org would populate based on the TARGET DIV https://trello.com/c/L0uTo8UG/3798-enhancement-manage-org-members-page-for-promotion-consider-adding-from-and-to-divisions-instead-of-just-one-division

So you would have: Program Source Div Source Target Div Target

I would further suggest that if Target Div is "(not specified)" then the Target Org selector populates based on Source Div. (but it this complicates things then it could just wait until Target Div is selected.)

hkouns commented 8 years ago

I dont know if it is due to the ReBasing (and managing conflicts...) but I am getting a error on the loading of the page now:

JavaScript critical error at line 148, column 1 in http://localhost:51611/Content/touchpoint/js/org/org-members.js\n\nSCRIPT1002: Syntax error
nehresma commented 8 years ago

Looks like rebasing did mess with things a bit. Rather than try to sort it all out I'm just going to create a new branch off of our master and cherry pick my commits over to it. Once I have a the new branch created I'll go ahead and delete this one.

For what it's worth, rebasing rewrites the git history so on a shared branch merge commits are usually preferred since they don't overwrite things.

hkouns commented 8 years ago

Sorry @nehresma ... I shouldn't have messed with it. I just knew the branch was pretty old and we needed to update. I think David likes the rebase because it doesn't have all the extra commits in it from updating.

nehresma commented 8 years ago

I've recovered most of it in the new org_member_management_changes branch. I haven't tested that branch yet, it likely has issues. I still have a little work to do:

I've gotta run right now but I will work on the back end changes for the member type change over the weekend.

nehresma commented 8 years ago

No worries. Yeah the branch was indeed taken from a very old point. I didn't realize that and so it was my fault.

Yeah I know some people prefer rebasing because of the merge metadata commits. It just makes it tricky for multiple people to work on the same branch if they're both rewriting history and pushing their changes. The TouchPoint team may be small enough that it doesn't really affect them though.

hkouns commented 8 years ago

Sounds good... What are your thoughts on the "Target Div" selector?

nehresma commented 8 years ago

Should be doable. I will look at that once the change member type on move is completed.

On May 19, 2016 9:51:03 PM Heath Kouns notifications@github.com wrote:

Sounds good... What are your thoughts on the "Target Div" selector?


You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/GSBCfamily/bvcms/issues/7#issuecomment-220498050

nehresma commented 8 years ago

So I was wrong about the member movement being done in Python. It is just a C# class with python in the name for some reason. Heh. Anyway, changing the member type while moving the person is now wired up on the back end.

The org_member_management_changes branch was branched from our master. When we're done making changes to it and we're ready to create a pull request into the bvcms repo, I'll rebase it off of their master since David prefers that.

I will work on the target div thing next.

nehresma commented 8 years ago

Target Division selector support is in the org_member_management_changes branch. Here's a screenshot.

image

I've been testing things here locally, but if you have time and could do some click throughs I'd appreciate another set of eyes on all the changes in this issue. Once we're happy with things, I'll rebase off of their master and create a pull request into the bvcms repository.

Thanks!

hkouns commented 8 years ago

@nehresma, I have done some local testing and things appear to be going well. Thanks for your work on this. I made a couple minor layout changed and submitted a pull request. (https://github.com/GSBCfamily/bvcms/pull/88) let me know what you think. Here is a sceenshot: org_mng_layout

Also, I wonder if we should reconcile the fact that the grades box uses COMMA separated values while sub-group uses SEMI-COLON?

I was going to go ahead and load to the Test Site... but it appears my IP has changed, so I have asked David to modify the settings on the remote server.

nehresma commented 8 years ago

Merged. Nice work!

Good call about making the separator consistent between grades and sub-groups. I used semicolon on sub-group because I thought there was a higher likelihood that a sub-group might contain a comma in the name and we wouldn't want to split on that. Do you think it would it be ok to use semicolon for grade too? Actually, I could make grade split on comma OR semicolon in case one feels more natural, but again that would differ from sub-groups.

Thoughts?

hkouns commented 8 years ago

@nehresma Thanks... I thought I recalled this from last night... but I just tested again. The grades work whether you use Comma OR Semi-colon.

If possible, I would suggest that we:

hkouns commented 8 years ago

@nehresma , I got the code loaded to the Test site (one of the configuration files had been over written.) I had been expecting the Sub-group filters to all be applied (AND query) however, you correctly coded it to be similar to the way the Organization Page filters worked which is an OR query for each subgroup. However, the Org Page also offers the ability to add a "Match All" condition (among others) to the query. Is that possible in our case?

To observe the difference... log onto the test site: https://gsbcfamily.tpsdb.work/Org/68

Thoughts?

hkouns commented 8 years ago

@nehresma
FYI... before doing PR to BVCMS we should make sure that the javascript files that have been modified is not part of a bundled file... if so, we will need to compress and Minify...this has bitten me before (on email replacement codes ) They use Gulpfile.js ... and use option "compress-js"

Here is a comment from a trello card: "That javascript file you modified is part of a bundled/minified file. I did not re-bundle and minify your changes. This is done in Visual Studio with the Task Runner Explorer window and uses the gulpfile.js specification file. " https://trello.com/c/xOxPPU8T/3527-suggestion-warning-for-single-email-address-revision

nehresma commented 8 years ago

@hkouns I will do the semicolon and comma changes tonight. I will also work on changing the query predicate logic between AND and OR depending upon the state of the "Match All" checkbox.

Good call about the javascript changes too. I disable the uglify gulp pipe step on my local Visual Studio environment so that I get readable JS to debug in the browser. I will look into this and commit those changes too.

FYI, we are going to be out of town Wed-Fri to take the boys to the Smithsonian museums in DC. If we have things in a decent state by tomorrow night I can create the PR before I head out of town if you want.

nehresma commented 8 years ago

@hkouns I'm looking at the semicolon thing in detail right now. I have a quick question regarding this: "modify sub-groups to split on Comma or semi-colon but continue to use semi-colons to separate as they are picked". If we split sub groups on commas then we would have problems when a sub group legitimately has a comma in the name. For example a sub group named "One, two, and three" would split out into 3 filters: "One", "two", "and three". None of which would cause the database match to return for the actual sub group named "One, two, and three".

Should we split sub-groups on comma anyway? I'm not sure how common it is to have a comma in the name of a sub-group (not just at our church but at others too).

I've pushed the changes for splitting on grades.

nehresma commented 8 years ago

Ugg, I should do more reading of the code before I type comments. Please disregard the previous question about adding comma as a separator to sub-groups. I believe I've figured out a workaround.

nehresma commented 8 years ago

Just pushed support for "Match all".

I have 2 more things to do here yet that I'm aware of (did I forget something?):

nehresma commented 8 years ago

@hkouns I've pushed the uglified JS as well as better age filtering. I've been clicking through and testing things and things seem to work, but if you have time to do another click-through at some point I'd appreciate it.

Thanks!

hkouns commented 8 years ago

@nehresma Thanks for all your help on this. I was just testing the previous commit and it seemed to be good. I will publish the latest to the work site. (Have you tried testing there?... there is a larger group of people to work with) You could probably go ahead and rebase and submit a PR... I think any changes would be minor. Have fun at the Museum.

nehresma commented 8 years ago

Cool I'll go ahead and 1) rebase the branch off of their master, 2) force push (this will make the branch's history timeline different from any local checkouts we have) and 3) create a PR.

I actually did try to get into the test site last night with my nathan@e........org account, but it wasn't an admin account so I could only view and edit my Person record.

nehresma commented 8 years ago

@hkouns FYI -- David made several changes to add a new "Move Registration Data" feature 2 days ago (https://github.com/bvcms/bvcms/commit/d219fc1a689ba0003bc5d69825c5c932349fa218). I've worked through merging our stuff with these changes.

I've also force pushed the branch after rebasing it from their master and created a PR to bvcms/master: https://github.com/bvcms/bvcms/pull/118 But beware that since I rebased and did a force push, the history timeline of the org_member_management_changes branch has been rewritten. So you'll likely need to do a force pull on any checked out copies of this branch (including the test server) and it will overwrite any local commits you have on that particular branch that weren't pushed at the time I rebased.

I'll keep an eye on this while I'm out of town but I will only have my laptop -- no Windows VM.

hkouns commented 8 years ago

@nehresma Karen has been testing the changes and verified all the specified items in Trello ()

She has the following suggestions/questions:

  1. Should we add a Clear button that will reset all the dropdowns and remove any text?
  2. I was wondering if we need a None option for the sub-groups - to find those not in a sub-group
  3. Should we have an error msg when someone clicks Move, but no one is selected? You have one if no Target is selected.

What do you think about the time required for these? I would like to use the features next week if possible.

nehresma commented 8 years ago

I can do all 3 of those. Probably about 2 hours of work. I wont be able to start until sometime over the weekend though.

hkouns commented 8 years ago

@nehresma I played around with it a bit and was able to incorporate 1 & 3 in a new branch. I submitted a new PR https://github.com/bvcms/bvcms/pull/119

nehresma commented 8 years ago

Awesome thanks!!

On May 26, 2016 2:35:02 AM Heath Kouns notifications@github.com wrote:

@nehresma I played around with it a bit and was able to incorporate 1 & 3 in a new branch. I submitted a new PR https://github.com/bvcms/bvcms/pull/119


You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/GSBCfamily/bvcms/issues/7#issuecomment-221789294

hkouns commented 8 years ago

I split the new features above into a new Issue... so that this one can be closed when the current PR to BVCMS is published.

hkouns commented 8 years ago

Closed with Merged PR https://github.com/bvcms/bvcms/pull/118