Skip to content
This repository was archived by the owner on Oct 29, 2020. It is now read-only.

Reportback items endpoint#5055

Merged
chloealee merged 10 commits intoDoSomethingArchive:devfrom
chloealee:reportback-items-endpoint
Sep 10, 2015
Merged

Reportback items endpoint#5055
chloealee merged 10 commits intoDoSomethingArchive:devfrom
chloealee:reportback-items-endpoint

Conversation

@chloealee
Copy link
Copy Markdown
Contributor

Fixes DoSomethingArchive/api#7

What's this PR do?

Adds a user's first_name, last_name, country, and photo to the reportback-items endpoint. Makes the request from phoenix to ns to grab user data and caches this data for 24 hours.

Where should the reviewer start?

phoenix -> northstar call is made in lines 132-148 of ReportbackController.

How should this be manually tested?

Comment in line 137 of ReportbackController.php to get a specific user's information.
User endpoint dev.dosomething.org:8888/api/v1/reportback-items.json

To test caching is working correctly: http://drupal.stackexchange.com/questions/87184/how-to-check-that-cache-is-working-on-current-page

What are the relevant tickets?

Will this conflict @aaronschachter?
DoSomethingArchive/LetsDoThis-iOS#286

@chloealee chloealee assigned chloealee, jonuy and weerd and unassigned chloealee and jonuy Sep 9, 2015
@chloealee
Copy link
Copy Markdown
Contributor Author

@uy @weerd ready to be looked at whenever either of you get a chance!

@aaronschachter
Copy link
Copy Markdown
Contributor

@chloealee No need to worry about DoSomethingArchive/LetsDoThis-iOS#286 for this PR, thanks for checking!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

was NORTSTAR_URL not working locally?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the URL was sending me to qa instead of local where I wanted to test but looks good now so I'll change back!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I actually changed this back - right now, the $base_url prints http://northstar.dev:8000 where as before, it printed http://northstar-qa.dosomething.org:8000. Is this ok?

@chloealee
Copy link
Copy Markdown
Contributor Author

I'm also worried about the cache actually working - is there a way to test this on staging?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

that's from pulling from stage, you can reset that locally here

@angaither
Copy link
Copy Markdown
Contributor

@chloealee as far as cache testing, you can see that in the cache tables in sql pro. I can help ya with that if you want 😄
also... since this is a new cache, I think you actually have to make the table with an install hook like this

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not vital at all (or maybe just a future update to consider), but might be cleaner if you could save the nested array to $northstar_user and then cast it to an object:

$northstar_user = json_decode($northstar_user->data, true);
$northstar_user = (object) $northstar_user['data'][0];

So then you can get each item similar to how code above retrieves it for consistency:

$this->user = [
  'id' => $data->uid,
  'first_name' => $northstar_user->first_name,
  'last_name' => $northstar_user->last_name,
  'photo' => $northstar_user->photo,
  'country' => $northstar_user->country,
];

Something like that... :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

that looks a lot better, I'll update now, thanks Diego!

Comment thread dosomething_northstar.install Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

💣 meh, looks like we don't need this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ah yes, sorry for being so sloppy and missing this! Removed.

@angaither
Copy link
Copy Markdown
Contributor

👍 🚢 🍷

chloealee added a commit that referenced this pull request Sep 10, 2015
updates `/reportback-items` endpoint with user's `first_name`, `last_name`, `country`, and `photo`
@chloealee chloealee merged commit 6daed57 into DoSomethingArchive:dev Sep 10, 2015
@chloealee chloealee deleted the reportback-items-endpoint branch September 10, 2015 20:46
@weerd
Copy link
Copy Markdown
Contributor

weerd commented Sep 10, 2015

Nice work @chloealee 💃

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants