search

saxifrage / cityasacampus

An open-source platform for connecting and showcasing resources within local learning communities.
http://cityasacampus.org/
5 stars 4 forks source link

edit grids and pathways #384

Closed chadwhitacre closed 8 years ago

chadwhitacre commented 8 years ago

Incorporates #377, so merge that first and then rebase here.

To-do

chadwhitacre commented 8 years ago

yb3htrjjec

chadwhitacre commented 8 years ago

What happens if anon tries to edit?

chadwhitacre commented 8 years ago

I'm having trouble testing as anon. I can't find localhost:3000 from curl, even with --ipv4.

$ curl http://localhost:3000/
curl: (7) Failed to connect to localhost port 3000: Connection refused
$ curl --ipv4 http://localhost:3000/
curl: (7) Failed to connect to localhost port 3000: Connection refused
$

Postman (h/t @dmtroyer) gives a 500:

screen shot 2015-12-21 at 1 20 04 pm

Is that the anon behavior, then?

chadwhitacre commented 8 years ago

I yanked the Cookie from Chrome (which encodes auth, ya?) and put it in Postman and I still get the same 500.

chadwhitacre commented 8 years ago

Gosh. Rails and Chrome are using IPv6! :flushed:

screen shot 2015-12-21 at 1 35 10 pm

$ curl --ipv4 http://localhost:3000/
curl: (7) Failed to connect to localhost port 3000: Connection refused
$ curl --ipv6 http://localhost:3000/
<!DOCTYPE html>
<html ng-app="caac">
  <head>
    <meta charset="utf-8">
    <meta http-equiv="X-UA-Compatible" content="IE=edge">
    <meta name="viewport" content="width=device-width, initial-scale=1">
    <link rel="shortcut icon" href="shared/favicon/favicon.ico"/>
    <title></title>
    <link rel="stylesheet" href="app.min.css">
    <link rel="stylesheet" href="vendors.min.css">
    <link rel="stylesheet" href="http://fonts.googleapis.com/css?family=Open+Sans:400,400italic,600,600italic,800,800italic">
    <script type="text/javascript" src="//maps.googleapis.com/maps/api/js"></script>
  </head>
  <body>
    <ng-view></ng-view>
    <script type="text/javascript" src="vendors.min.js"></script>
    <script type="text/javascript" src="app.min.js"></script>
  </body>
</html>
$
chadwhitacre commented 8 years ago

Okay! Rails wants an application/json POST body, I had been sending form-data from Postman.

Looks like the PUT still works without the Cookie header, which I take it indicates that we aren't sufficiently protecting the endpoint yet.

screen shot 2015-12-21 at 1 37 42 pm

chadwhitacre commented 8 years ago

Yeah, these endpoints are wide open:

$ curl --ipv6 http://localhost:3000/pathways/1.json
{"pathway":{"id":1,"name":"Bloo","nodes":[]}}$
$ curl --ipv6 http://localhost:3000/pathways/1.json -X PUT --data-binary '{"name": "Blah"}' -H"Content-Typepplication/json"
$ curl --ipv6 http://localhost:3000/pathways/1.json
{"pathway":{"id":1,"name":"Blah","nodes":[]}}$

Hmm ... I guess we need to fix that app-wide, not just here. I think I'll leave that to another PR for now.

chadwhitacre commented 8 years ago

POSTing to pathways also works for anon:

$ curl --ipv6 http://localhost:3000/pathways.json -X POST --data-binary '{"name": "Flam", "grid_id": "1"}' -H"Content-Type: application/json"
{"pathway":{"id":2,"name":"Flam","nodes":[]}}$
chadwhitacre commented 8 years ago

8z066ekmgj

dmtroyer commented 8 years ago

Move the endpoint within the api/v1 scope.

dmtroyer commented 8 years ago

OH! And in the rails controller add a call to authenticate_user! in a before_action filter for the pertinent actions ala https://github.com/saxifrage/cityasacampus/blob/edit-map-things/app/controllers/organizers_controller.rb#L3

dmtroyer commented 8 years ago

uhh, I can make those changes on #377 before I merge it?

chadwhitacre commented 8 years ago

bxcw7ph3o9

chadwhitacre commented 8 years ago

uhh, I can make those changes on #377 before I merge it?

Whichever. #377 is just against #364, so merging #377 is not merging to master.

dmtroyer commented 8 years ago

Whichever. #377 is just against #364, so merging #377 is not merging to master.

Ok, I'm a little confused with the PR hierarchy so I'll just put it somewhere.

chadwhitacre commented 8 years ago

I'm a little confused with the PR hierarchy

Yeah, it's gettin' squirrelly. :-)

My expectation is that we would merge #377 to #364 next. Then I'll rebase this PR and get it ready for review & merge to #364 as well.

I'm expecting to keep making smaller PRs against #364 until #364 is ready to merge to master.

chadwhitacre commented 8 years ago

380 is ready for review and merge to master.

377 is ready for review and merge to #364.

chadwhitacre commented 8 years ago

I'll just put it somewhere

I propose that we make another PR against #364 to clean up auth for all of the endpoints in one fell swoop.

chadwhitacre commented 8 years ago

Do we have any modals lying around that we could use instead of the stock browser UI popups?

chadwhitacre commented 8 years ago

Nothin' called modal in client/.

chadwhitacre commented 8 years ago

Not gonna worry about modals for now.

chadwhitacre commented 8 years ago

Onward!

chadwhitacre commented 8 years ago

Rebased onto #364 at d611e9cd741d80bbec717f7e9c0320fa36bb37fb. Previous head was 1c5c0ef8ab7bd659ad5179292d6fa1f4886d2e31.

chadwhitacre commented 8 years ago

Squashed. Previous head was b17d391513b3ba158ae53e30c0aa3ec8bcc0da66.