Skip to content
This repository has been archived by the owner on Jun 24, 2022. It is now read-only.

Add school and district admins #63

Conversation

carols10cents
Copy link
Contributor

Hi! This adds support for syncing school and district admins through the api. The code changes are a little bit icky because these resources (especially district admins) are different than the other resources-- I tried to document either in comments or the commit messages when I had to do something inconsistent with the existing code because of these differences.

Please let me know about any feedback you have.

The district response does not have links to the district_admins and
school_admins URLs, so we need to hard code them instead of using
@linked_resources.
Because we want DistrictAdmin to become district_admin instead of
districtadmin.
So that school_admins and district_admins are valid class names
extracted from the URLs.
The district admins resource doesn't support the `limit` parameter, so
just remove it from this test.
District admin records don't have links or URIs, and they don't have the
inner level of data.
The district admin resources don't support the list parameters.
Add new requests for district and school admins.
Update requests to include newer headers and encodings.
Add documentation for the way I made these updates.
@@ -59,6 +65,16 @@ $ rake lint

Running specific tests is not currently supported.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the above mean that this line is now untrue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I just added the instructions to run one test file; the only way I know of to run one specific test with minitest is the -n option (--name PATTERN Filter run on /pattern/ or string.) which I find annoying. When I want to do this, I usually bring m into the picture, but I didn't want to add another dependency if you didn't want it :)

* Remove the existing VCR cassettes that you would like to update so that they will get rerecorded.
* Run the tests.
* Using a combination of `git checkout -p`, `git add -p`, find-and-replace, `sed`/`awk`, etc. check into git the updated data and any updated formatting that you want to keep. **Check out and discard any recordings of your `Bearer Token` (so that the cassettes all contain `DEMO_TOKEN`)** and any other irrelevant data changes. Change the IDs of any objects in your added or updated response data to match those of the demo data.

Copy link
Contributor

Choose a reason for hiding this comment

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

We'll need to have a better process than this in the future; #64

@osdiab
Copy link
Contributor

osdiab commented Sep 28, 2015

Aside from the above comments the approach lgtm :)

@carols10cents
Copy link
Contributor Author

I was out of town last week; I'll be back on this PR this week :)

It's sounding like you'd rather have a pull request with only the fully supported SchoolAdmin right now, and not support DistrictAdmins in ruby for the time being?

@carols10cents
Copy link
Contributor Author

And first on my list will be rebasing this on master, of course.

@carols10cents carols10cents mentioned this pull request Oct 5, 2015
@carols10cents
Copy link
Contributor Author

Closing in favor of #69 .

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

Successfully merging this pull request may close these issues.

2 participants