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

More universal membership statuses #15

Open
jamstooks opened this issue Feb 10, 2017 · 5 comments
Open

More universal membership statuses #15

jamstooks opened this issue Feb 10, 2017 · 5 comments

Comments

@jamstooks
Copy link
Contributor

jamstooks commented Feb 10, 2017

It looks like these membership statuses are custom to our implementation.

Maybe we should have some way for the user of the package to define his/her own status dict? Or, should we collect the statuses from MemberSuite in some way?

The problem with collecting them is that this could affect performance if we have to do it frequently.

@jamstooks
Copy link
Contributor Author

@rerb and @baronvonvaderham - I'm leaning toward having the user of the package define the status and type dictionaries (and maybe other custom values), but I'd be interested in your thoughts.

@baronvonvaderham
Copy link
Contributor

We had 103 types of memberships, so I set that like the metadata classes on the hub. We could do that here, but I feel like this is small enough and doesn't change frequently enough to add more complexity to the ISS sync.

We could have it fed in from your settings or something. Then here we just save those weird IDs into the field, translate it on the django app (in our case ISS) when saving it into the DB model.

@rerb
Copy link
Contributor

rerb commented Feb 10, 2017

Define (and share) custom constants outside membersuite_api_client. Define in a package we can share, maybe like this:

from membersuite_custom_constants import ORG_TYPES, MEMBER_STATUSES

That's my vote.

@jamstooks
Copy link
Contributor Author

I like that idea, @rerb. It's a bit like the approach we took with django-integration-settings. How others implement these dictionaries would be up to them.

@baronvonvaderham - I'm wary of relying too heavily on the assumption that users of this API will be using Django. If it does get used, then I suspect that many may use Django, but others may just want pure python options.

I think we can hold on this for now and make the design decision after we have more modules and have used this more. We may want to extract these dictionaries to another package like aashe-membersuite-constants or something with broader application. I could even see some utils to pull these values from Membersuite in that package or even the client package so they could be stored in a repo or Django model.

@rerb
Copy link
Contributor

rerb commented Feb 11, 2017

I could even see some utils to pull these values from Membersuite in that package or even the client package so they could be stored in a repo or Django model.

+1 on loading the config from MemberSuite and storing somehow. How about this process, run as part of initial configuration and whenever any of those enum values change, generates the Python code that defines the constants, say, custom_constants.py? Don't have to store the values in a DB if they're in the code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants