Skip to content
This repository has been archived by the owner on Feb 23, 2021. It is now read-only.

Let the marker show up even when not locked in #35

Closed
albertinisg opened this issue Mar 4, 2016 · 12 comments
Closed

Let the marker show up even when not locked in #35

albertinisg opened this issue Mar 4, 2016 · 12 comments
Assignees
Labels
Milestone

Comments

@albertinisg
Copy link
Collaborator

Literally reported by a user:

"Let the marker show up even when not locked in, this is a unusual behavior for me. Prevent to edit, yes, but not showing?"

@albertinisg albertinisg self-assigned this Mar 7, 2016
@albertinisg
Copy link
Collaborator Author

The actual approach is that all the requests against TourGuide must be authenticated using a token from Keyrock. @FiwareULPGC @jmcanterafonseca, should we change the actual approach and let the user get restaurants, reviews and reservations without being logged in? Take into account that if doing this, we should also disable the edit/create buttons, hide the sections 'my reservations' and 'my reviews' and so on.

@jmcanterafonseca
Copy link
Collaborator

I would keep this issue open and let analyze it later as we have other priorities

@albertinisg albertinisg added this to the Sprint 5.3.1 milestone Mar 30, 2016
@albertinisg
Copy link
Collaborator Author

I think we should get back to this. Guys, what are your thoughts? @FiwareULPGC, would this be a huge change in the client? From the server side, the change shouldn't be a big deal.

Still, IMHO, the actual approach makes the user interacts with the Log-in - oauth - IdM authorization, and this change can make the user run the application and not interact with that feature.

@FiwareULPGC
Copy link
Contributor

Hi!

We think that it would not be too complicated to adapt the client to the proposed approach.

If we are going to implement it in a close future, if it is possible, it should be nice to know it in advance, to take it in care for the next planned client refactor and avoiding to refactor the client twice.

Thank you!

@albertinisg
Copy link
Collaborator Author

Thanks for the feedback! It was planned to be done in this sprint, but I'm afraid is too bit late (we have one week left for this sprint). Do you think is it feasible? If not, @jmcanterafonseca, should we move it to the next sprint?

@jmcanterafonseca
Copy link
Collaborator

yup

2016-04-21 19:08 GMT+02:00 Alberto Martín notifications@github.com:

Thanks for the feedback! It was planned to be done in this sprint, but I'm
afraid is too bit late (we have one week left for this sprint). Do you
think is it feasible? If not, @jmcanterafonseca
https://github.com/jmcanterafonseca, should we move it to the next
sprint?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#35 (comment)

@albertinisg
Copy link
Collaborator Author

@FiwareULPGC, sever change for this issue is done #92 (waiting for review)

@FiwareULPGC
Copy link
Contributor

FiwareULPGC commented May 5, 2016

We have tested your branch (added as remote and merged with a new branch from develop), and the client can't get information if the user is not logged in.

We have checked that the last commit is 98f96ed (the same that the PR).

Here we detail the steps that we use to test the PR:

Get PR:

  • git remote add albertinisg https://github.com/albertinisg/tutorials.TourGuide-App.git
  • git fetch --all
  • git checkout develop
  • git pull
  • git checkout -b test-pr96
  • git merge albertinisg/feature/allow-get-all

Build the docker image:

  • docker build -t tourguide-pr92 .

Modify tourguide image:

tourguide:
    image: tourguide-pr92

Launch the tourguide app:

  • docker-compose pull
  • docker-compose up

Access to the application using Firefox:

Make a request (without log in):

  • AJAXRequest.get('http://tourguide/api/orion/restaurants/', function(data){console.log(data)}, function() {alert('Could not retrieve restaurants');});

The error message is raised.

Log in and repeat the request:

  • AJAXRequest.get('http://tourguide/api/orion/restaurants/', function(data){console.log(data)}, function() {alert('Could not retrieve restaurants');});

The response with all restaurant data is printed in the console.

@albertinisg
Copy link
Collaborator Author

The image is not built (yet) in dockerhub with the changes, so you need to add the changes mapping the host in the docker-compose file (using volumes):

tourguide:
    image: fiware/tutorials.tourguide-app:latest
    hostname: tourguide
    links:
        - orion
        - idm
        - pepwilma
        - idasiotacpp:idas
    environment:
        - ORION_HOSTNAME=pepwilma
        - ORION_NO_PROXY_HOSTNAME=orion
        - ORION_PORT=1026
        - ORION_PEP_ENABLED=true
        - IDAS_PORT=8080
        - ORION_SUBSCRIPTIONS_ENABLED=false
        - SENSORS_GENERATION_ENABLED=false
        - SENSORS_FORCED_UPDATE_ENABLED=false
    volumes_from:
        - idm
    volumes:
      - ../../../tutorials.TourGuide-App:/home/tourguide/tutorials.TourGuide-App

@FiwareULPGC
Copy link
Contributor

We have followed your indications and we could test it properly (after installing grunt). The client can get information even if the user is not logged in.

We also tested forbidden operations like delete and they returned 404 errors (maybe you could consider to return 403 errors instead)

Apart from the returned error codes it LGTM.

@albertinisg
Copy link
Collaborator Author

I'll open an issue to check that and re-asign this one to @FiwareULPGC . Thanks for the feedback!

@FiwareULPGC
Copy link
Contributor

Implemented via #117

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

No branches or pull requests

3 participants