Add user profile page #728

asaunier · 2016-10-12T18:50:25Z

Fixes #335
Fixes #640

User profile contains:

QUESTIONS:

~~have a profile list page (advanced search page)?~~
support of v5 URLs (users/234567/fr) in addition to v6 URLs (profiles/234567/fr)? or done with other v5>v6 redirections?
remove the comments in user profiles? See No need for comments in profile pages v6_forum#47

asaunier · 2016-10-14T13:14:06Z

PR still WIP but updated.

User profile with no geometry (=> no map):

User profile with geometry (but no content):

Despite what is suggested in #335 (comment)

The ui server-side renders the content as HTML.
On the client-side you insert the received HTML in the page.

I have chosen to make the UI service return a JSON response instead of an HTML response. I wanted to avoid the problem we have with the preview service: angular code seems to be not executed in HTML code returned asynchronously. See #168 (comment)

The JSON data are then rendered using angular expressions.

There are several problems remaining though:

the description and summary attributes are anyway parsed server-side to convert the bbcode and markdown syntax to HTML. But I have not succeeded in preventing angular from escaping this HTML code (despite a "trustAsHtml" filter). I think I had the same problem with the preview service and had to use a jquery workaround...
the userProfile directive (created by this PR) seems executed before the map directive. Thus the user feature is sent to the map before it is ready. As a result the map is not recentered correctly.
it's a bit harder to build the profile content because I cannot use the mako helpers. I wonder if I won't have to go back to an HTML response of the UI user profile service (instead of JSON).

tsauerwein · 2016-10-14T13:21:45Z

I would be great to use the design that was planned for the profiles (with the nice banner image at the top):
https://drive.google.com/file/d/0B3v6NzlMgPYlaWtfTFJHZ3hibHM/view

tsauerwein · 2016-10-14T13:22:54Z

I have chosen to make the UI service return a JSON response instead of an HTML response. I wanted to avoid the problem we have with the preview service: angular code seems to be not executed in HTML code returned asynchronously.

That's a problem we have to solve anyway. This should not be an argument to do it differently for the profile.

asaunier · 2016-10-14T13:27:59Z

I would be great to use the design that was planned for the profiles (with the nice banner image at the top):

What nice banner image?
There's no such banner.
It would mean people should be able to upload their own image banner. But it's out of our scope.

The rest of the page is completely irrelevant here: it's the user's prefs UI, which is another page.

asaunier · 2016-10-14T13:29:34Z

That's a problem we have to solve anyway. This should not be an argument to do it differently for the profile.

You are merciless :P

tsauerwein · 2016-10-14T13:30:18Z

What nice banner image?
There's no such banner.
It would mean people should be able to upload their own image banner. But it's out of our scope.

For the long term it would be nice that the user can upload their own banner image. But for now a default image would already be good. Or a random image out of 3-5. Maybe @ginold can do some magic here like for the homepage? :)

The rest of the page is completely irrelevant here: it's the user's prefs UI, which is another page.

But the header is the same for the profile page.

tsauerwein · 2016-10-14T13:32:18Z

You are merciless :P

:D Sorry, but you self are saying:

it's a bit harder to build the profile content because I cannot use the mako helpers. I wonder if I won't have to go back to an HTML response of the UI user profile service (instead of JSON).

asaunier · 2016-10-14T13:34:15Z

For the long term it would be nice that the user can upload their own banner image. But for now a default image would already be good. Or a random image out of 3-5

It does not make sense to display a dummy image (that would be the same for every one or a random one not even chosen by the user). And we simply don't have time for this.

tsauerwein · 2016-10-14T13:34:55Z

It does not make sense to display a dummy image (that would be the same for every one or a random one not even chosen by the user).

It would make the page look better...

asaunier · 2016-10-14T13:38:34Z

It would make the page look better...

For sure!

But if the user cannot choose their own image (and we don't have time for this), they won't be happy with it. So it's better to have no image in the meantime.

Honestly we already have enough troubles with this page...

tsauerwein · 2016-10-14T13:44:47Z

But if the user cannot choose their own image (and we don't have time for this), they won't be happy with it. So it's better to have no image in the meantime.

I think differently about this.

Honestly we already have enough troubles with this page...

Then let's first take care of these troubles and we can discuss the banner image again once they are solved.

ginold · 2016-10-17T06:29:31Z

+1 for the image banner! it adds so much to the page. The not-logged user page looks so much nicer with an image - could we use the same image for everyone for now (the one that is found on the mock-ups) ?

...when the troubles are solved :P

asaunier · 2016-10-17T09:25:30Z

I have the feeling that the current way this page is built (with an additional request to the API to add the content - whether returning HTML or JSON) is a bit too complicated and might cause problems, for instance to inject the additional HTML or make the additional angular code be executed (see the problems with the preview service).

I wonder if we could not use the standard way to generate a document detail view. The tricky thing is to manage the case where the profile page is not public.
Would it be possible:

either to directly pass the Authorization header (with the JWT token) if available when the browser requests the profile page to the UI (which would then pass it to the API)
or to make a standard detail view request (with no JWT token) => if the profile is public it would then be returned as usual ; if not public, the API would return a 401/403 response to the UI server. Would it be possible then that the UI server asks the browser for the JWT token, gets it and makes a new API request this time with the JWT token?

This way we could keep the usual way of generating a document detail view page, with the usual tools => no problem to integrate the map info or the other additional content.

What do you think?

tsauerwein · 2016-10-17T09:47:45Z

either to directly pass the Authorization header (with the JWT token) if available when the browser requests the profile page to the UI (which would then pass it to the API)

The browser makes the request, so you can not set custom headers.

Would it be possible then that the UI server asks the browser for the JWT token...

How would that work?

asaunier · 2016-10-17T09:56:10Z

How would that work?

If I knew I wouldn't ask :P
I'm googling a bit to see if there are such solutions.

asaunier · 2016-10-17T10:16:16Z

Perhaps it could be done like this:

the browser requests users/123456/fr to the UI as usual (with no JWT token)
the UI makes a request to the API. If the profile is public, the API returns the data and the UI builds the detail view page as usual.
if the profile is not public, the API returns an error, the UI then serves a fallback page providing the basic profile info (user name) and a message saying that the profile is not public.
the fallback page also contains a directive (or directly a controller?) that checks whether the user is authenticated or not. If not, it displays a message "please log in to view this page", or even directly redirects to the login page?
If the visitor is authenticated, the controller makes an additional users/123456/fr request to the UI, this time passing the JWT token and eventually gets the profile page.

Perhaps we should also consider making newly created profile pages (for new registered users) public by default.

An other approach:

always display the profile page attributes (if it's a problem, people may edit the page and remove the content). Most of the time data in the profile page attributes are not too personal: we don't show emails, we don't collect personal data such as date of birth, gender or phone numbers, and the user name (topo name and forum name) is anyway public.
only use the "public_profile" info to figure out if the user feed should be displayed or not (I think that's the main issue with profile being public: people may be reluctant that anyone can see their list of outings and contributions.

tsauerwein · 2016-10-17T11:19:37Z

If the visitor is authenticated, the controller makes an additional users/123456/fr request to the UI, this time passing the JWT token and eventually gets the profile page.

Why not always do it like this? Currently only 7% of the profiles are public. Why make it more complex for a less common case?

always display the profile page attributes (if it's a problem, people may edit the page and remove the content). Most of the time data in the profile page attributes are not too personal: ...

only use the "public_profile" info to figure out if the user feed should be displayed or not ...

This should be up to the user to decide. Information that was private in v5 should also be private in v6.

asaunier · 2016-10-17T11:26:07Z

Why not always do it like this?

To do what? Passing the JWT token?
You just told me it's not possible!?

Or perhaps I don't understand your point, could you precise?

tsauerwein · 2016-10-17T11:29:05Z

If the visitor is authenticated, the controller makes an additional users/123456/fr request to the UI, this time passing the JWT token and eventually gets the profile page.

I understood that you are talking about the original idea as described here: #335

asaunier · 2016-10-17T11:34:11Z

I understood that you are talking about the original idea as described here: #335

No, as I said the original idea is also complicated and not that easy to implement.

If the visitor is authenticated, the controller makes an additional users/123456/fr request to the UI, this time passing the JWT token and eventually gets the profile page.

I suggested that the controller in the "fallback page" would reload the full page (not make an asynchronous request to load a part of the page as currently) and pass the JWT token.
Not sure it's possible though because

When you use $window.location.href the browser is making the HTTP request and not your JavaScript code. Therefore, you cannot add a custom header like Authorization with your token value.
http://stackoverflow.com/a/26373493

asaunier · 2016-10-19T16:16:30Z

I'm having a hard time trying to recenter the map on the user's geometry (if any).

The tricky thing is to pass the user feature to the map directive. For other point document it is done by setting the mapFeatureCollection module value. For instance with waypoints: https://github.com/c2corg/v6_ui/blob/master/c2corg_ui/templates/waypoint/view.html#L43-L72

With the user profiles, it's a bit more complicated because the profile data (including the geometry) are retrieved AFTER the initial page has been generated (with the default moduleConstantsValues mako block, set to a null feature collection).

When the profile data are returned as some additional HTML code plugged and compiled in some place of the initial page, a map directive is actually added and rendered. But I still need to find a way to pass the geometry (most likely in a featureCollection as for the other document types).

I have tried to redefined the module value in the additional HTML code:

<script>
var module = angular.module('app');
module.value('mapFeatureCollection', {
        "type": "FeatureCollection",
        "properties": {},
        "features": [
        {
          "type": "Feature",
          "geometry": {"type": "Point", "coordinates": [${geometry.x}, ${geometry.y}]},
          "properties": {
            "title": ${dumps(profile['name']) | n},
            "lang": "${locale['lang']}",
            "documentId": ${user_id},
            "module": "profiles",
            "highlight": true
          }
        }
      ]});
</script>
<app-map class="view-details-map"></app-map>

but for some reason the map directive always receive the initial mapFeatureCollection module value. Perhaps because the "overridding" one is ignored because of something like http://stackoverflow.com/a/27122977 ?

I have then tried to pass the feature collection as an additional map directive attribute app-map-feature-collection. But I don't know how to pass a JS variable as a directive scope attribute (it seems one have to inject $window into the map controller, but that does not sound like a very elegant solution.

I have eventually tried this solution (using an expression to save the feature collection to a scope element): afd713e
but this is not very elegant either. In addition it works in debug mode but fails in build mode because

deps.js:157 Error: [$rootScope:infdig] http://errors.angularjs.org/1.5.8/$rootScope/infdig?p0=10&p1=%5B%5B%7B%22ms…22module%22%3A%22profiles%22%2C%22highlight%22%3Atrue%7D%7D%5D%7D%7D%5D%5D

I had a similar error in debug mode before adding the one-time binding operator :: to the expression.

Tips very much welcome :)

asaunier · 2016-10-19T19:00:03Z

Another complication: it seems that it's not possible to directly call controllers (ignored? not defined?) from the HTML added asynchronously in the profile page.

For instance tooltip translating as in

<span class="route-activity icon-mountain_climbing" uib-tooltip="{{mainCtrl.translate('mountain_climbing')}}"></span>

or checking the user permissions before showing the floating buttons (edit, add images, etc.):

<div ng-show="userCtrl.auth.isAuthenticated()">BBBBBBBBBB</div>

do not work.

On the other hand, directives (eg. map) seem to be taken into account as expected.

asaunier · 2016-10-19T21:14:03Z

Another complication: it seems that it's not possible to directly call controllers (ignored? not defined?) from the HTML added asynchronously in the profile page.

To limit this problem, 5fe23f1 adds the minimal data (eg. user name) to the base profile page (+ makes this page cached). The next step is to transfer the editing buttons to this base page (the additional HTML then containing only the actual user infos), this way controllers will be available.

tsauerwein · 2016-10-20T07:23:54Z

I'm having a hard time trying to recenter the map on the user's geometry (if any).

See 7abbc1a

asaunier · 2016-10-20T19:13:03Z

This PR can now be reviewed.
Almost all points listed at #728 (comment) have been addressed.

I am not sure profile images are correctly shown in the profiles. It seems the API does not return them anyway?

There a still some styling fixes to do but I will probably need help!

The user icon on the map is still with a wrong size and color. See Icons #390 (comment)
the map is a bit over the profile title (name)
the profile feed starts below the main content and the feed items are too narrow

tsauerwein · 2016-10-21T06:36:26Z

I am not sure profile images are correctly shown in the profiles. It seems the API does not return them anyway?

The images associated to a user profile are returned with c2corg/v6_api#439. Please make sure your API instance is up-to-date.

ginold · 2016-10-21T07:30:51Z

I will try to fix your css issues.

tsauerwein

Thanks, that is looking good!

Maybe @ginold can take a look at the HTML/CSS part?

tsauerwein · 2016-10-21T06:48:27Z

c2corg_ui/static/partials/follow.html

+       class="float-button float-followed" tooltip-placement="left"
+       uib-tooltip="{{'No longer follow' | translate}}">
+    <span class="glyphicon glyphicon-star"></span>
+    <p class="float-button-text" translate>No longer follow</p>


Maybe as text: "Unfollow"
And as tooltip: "Stop following this user" ?
(that's how it is on Twitter)

tsauerwein · 2016-10-21T06:49:55Z

c2corg_ui/static/partials/follow.html

+
+  <div ng-hide="followCtrl.followed" ng-click="followCtrl.toggle()"
+       class="float-button float-notfollowed" tooltip-placement="left"
+       uib-tooltip="{{'Follow' | translate}}">


Maybe as tooltip: "See the activity of this user in your feed" or "See notifications for this user"?

Your suggestion make sense.
But isn't it more consistent with the other tooltip to write "Follow this user" (or "contributor"). I agree that your suggestions are more explicit.
In twitter the buttons are (in french) "Abonné", "S'abonner", "Se désabonner". Perhaps "Subscribe" in english?

Perhaps "Subscribe" in english?

In English it's "Follow/Unfollow". Maybe it's not clear to every user what "following a user" means.

In English in Twitter it's "Follow", "Unfollow", "Following" and there's no tooltip.

Maybe it's not clear to every user what "following a user" means.

OK I will use "See the activity of this contributor in your feed"

tsauerwein · 2016-10-21T06:56:44Z

c2corg_ui/static/js/userprofile.js

+
+  var config = {
+    headers: {
+      'Accept': 'application/json'


Is this header required?

tsauerwein · 2016-10-21T06:58:51Z

c2corg_ui/static/js/userprofile.js

+  var url = '/profiles/data/{id}/{lang}'
+    .replace('{id}', this.userId.toString())
+    .replace('{lang}', this.lang);
+  var promise = $http.get(url, config);


Maybe add a comment:

"An authenticated request is made to the ui server to get the profile data as rendered HTML (profiles can be marked as non-public)"

tsauerwein · 2016-10-21T07:04:08Z

c2corg_ui/static/partials/feed.html

@@ -26,7 +26,7 @@ <h4 class="text-success text-center" translate ng-show="feedCtrl.busy">Loading f

    <p class="action-line">
      <span class="action" ng-cloak>
-        <b><a ng-href="todo">{{doc['user']['name']}}</a></b>
+        <b><a ng-href="/profiles/{{doc['user']['document_id']}}/{{doc['user']['locales'][0]['lang']}}">{{doc['user']['name']}}</a></b>


{{doc['user']['locales'][0]['lang']}}

I opened a ticket on the API (c2corg/v6_api#423) because I thought that you would not need the locales of a user profile when getting a user in the listing version. But the way you are doing it here, means that you actually need the locale, right?

Yes, or at least the available_langs of the profile

tsauerwein · 2016-10-21T07:07:17Z

c2corg_ui/templates/document/diff.html

@@ -20,7 +20,7 @@ <h2 class="text-center">${title}</h2><br>
        <a href="${request.route_url(
              module + '_archive', id=document_id, lang=lang,
              version=version1['version_id'])}"><span translate>Revision as of</span> {{'${1000 * version1['written_at']}' | date:'medium'}}</a><br/>
-        <span translate>by</span> <a href="#TODO profile for ${version1['user_id']}">${version1['name']}</a>
+        <span translate>by</span> <a href="${request.route_url('profiles_view_id', id=version1['user_id'])}">${version1['name']}</a>


Would it make sense to have the user locale here? Like in c2corg_ui/static/partials/feed.html?

Indeed the user locale is not available when retrieving the archived versions I think. That's why I use the "id only" link.
Having the user locale (but not the whole description etc.) would make sense to create the whole profile link.
But it's not very critical ("nice to have") because the "id only" link also works (but triggers additional requests to get the lang when loaded).

Ticket for the API is here: c2corg/v6_api#446

tsauerwein · 2016-10-21T07:08:37Z

c2corg_ui/templates/helpers/edit.html

-    view_url = request.route_url(model + 's_view_id_lang', id=id, lang=lang) if updating_doc else ''
-    index_url = request.route_url(model + 's_index')
+    if model == 'profile':
+        view_url = request.route_url(model + 's_view', id=id, lang=lang)


Why is this different for profiles?

Because profiles have no slug URLs and no index URLs.
Index URLs might be added later.

tsauerwein · 2016-10-21T07:09:35Z

c2corg_ui/templates/helpers/view.html

@@ -313,7 +313,7 @@ <h3 class="heading show-phone outings" ng-click="mainCtrl.animateHeaderIcon($eve
 <%def name="get_document_description(locale, doctype)">\
  % if locale.get('summary') or locale.get('description'):
    <div class="view-details-description col-xs-12  description">
-      % if not doctype == 'article':
+      % if doctype != 'article' and doctype != 'profile':


You could write if doctype not in ['article', 'profile']:

tsauerwein · 2016-10-21T07:14:14Z

c2corg_ui/views/profile.py

+            (id, lang), cache_document_detail, load_data, render_page,
+            self._get_cache_key)
+
+    def _get_profile(self, id, lang, old_api_cache_key=None):


Maybe rename this method to _get_profile_info, because you are not requesting the profile.

tsauerwein · 2016-10-21T07:37:53Z

c2corg_ui/views/profile.py

+    _API_ROUTE = 'profiles'
+
+    @view_config(route_name='profiles_view')
+    def profile(self):


I think it would be good to document how the profile page is constructed. Something like:

User profiles are a bit special, because users can mark their profile as non-public so that the profile data has to be requested with an authentication header. The request to get a profile page is made by the browser, so that the authentication header can not be set. That's why the profile page is constructed as follows:

The browser makes a request /profiles/123 to the UI server-side (without authentication header).

The UI server sides makes an unauthenticated request to the API to get the available locales of the profile (api.camptocamp.org/profiles/123/info)

The UI server-side returns a page containing only the user name.

On the UI client-side a request is made to the UI server-side to get the profile data as rendered HTML (e.g. /profiles/data/123/fr). If the user is logged-in, the request is made authenticated.

The UI server-side makes a request to the API to get the profile data (e.g. (api.camptocamp.org/profiles/123/fr)). If the request to the UI server-side was authenticated, the request to the API is also made authenticated.

On the UI client-side the rendered HTML is inserted into the page.

tsauerwein · 2016-10-21T09:35:22Z

Related to #756, could you please make sure to use relative urls?

asaunier · 2016-10-21T10:51:27Z

I have just rebased/fixed conflicts and pushed the branch again.

Most of the comments made by @tsauerwein has been addressed.

I will make the "path" changes as suggested here:

Related to Use relative URLs instead of absolute for static assets #756, could you please make sure to use relative urls?

@ginold

It seems the "personal feed" switch on the home page is broken after I have resolved some conflicts.
feel free to make the style fixes, thanks!

ginold · 2016-10-21T13:13:15Z

second branch created because of conflicts : #768

asaunier mentioned this pull request Oct 13, 2016

User profiles access rights c2corg/v6_api#425

Closed

asaunier force-pushed the user-profile branch 2 times, most recently from 473cbfb to 73cfc9b Compare October 14, 2016 13:00

asaunier force-pushed the user-profile branch 2 times, most recently from f819b32 to 6a2ad19 Compare October 19, 2016 13:49

asaunier mentioned this pull request Oct 19, 2016

No need for comments in profile pages c2corg/v6_forum#47

Closed

asaunier added 3 commits October 20, 2016 13:50

Rename users view to profiles

a082382

Recenter the user profile map if a geometry is available

f9192d5

Fix user profile links

8b221bd

asaunier force-pushed the user-profile branch 2 times, most recently from e43907c to c3c192f Compare October 20, 2016 12:25

ginold mentioned this pull request Oct 20, 2016

User profile #335

Closed

asaunier mentioned this pull request Oct 20, 2016

Auth vs profile-feed c2corg/v6_api#441

Closed

asaunier added 5 commits October 20, 2016 19:00

Pass default data to base profile page + cache it

3db69e2

Add feed to user profile

5e9cd88

Remove non existing icons for areas

2376d56

Add editing of profile documents

bc33065

Add areas to profile + remove deprecated pin tag

1d76df5

asaunier force-pushed the user-profile branch from 168998e to 1d76df5 Compare October 20, 2016 17:11

asaunier changed the title ~~[WIP] Add user profile page~~ Add user profile page Oct 20, 2016

Add follow directive

98d4f3d

tsauerwein approved these changes Oct 21, 2016

View reviewed changes

This was referenced Oct 21, 2016

Include user locale for history and version web-service c2corg/v6_api#446

Closed

Check that locales of user profile are not included in query for listings c2corg/v6_api#423

Closed

asaunier force-pushed the user-profile branch from f43db64 to 0d37d11 Compare October 21, 2016 10:48

ginold force-pushed the user-profile branch from c212a46 to 98d4f3d Compare October 21, 2016 13:08

ginold mentioned this pull request Oct 21, 2016

User profile 2 #768

Merged

asaunier closed this Oct 21, 2016

asaunier deleted the user-profile branch October 21, 2016 15:14

asaunier mentioned this pull request Oct 21, 2016

Images associated to user profiles #770

Closed

Add user profile page #728

Add user profile page #728

Conversation

asaunier commented Oct 12, 2016 • edited by ginold Loading

asaunier commented Oct 14, 2016

tsauerwein commented Oct 14, 2016 • edited Loading

tsauerwein commented Oct 14, 2016

asaunier commented Oct 14, 2016

asaunier commented Oct 14, 2016

tsauerwein commented Oct 14, 2016

tsauerwein commented Oct 14, 2016

asaunier commented Oct 14, 2016

tsauerwein commented Oct 14, 2016

asaunier commented Oct 14, 2016

tsauerwein commented Oct 14, 2016

ginold commented Oct 17, 2016 • edited Loading

asaunier commented Oct 17, 2016

tsauerwein commented Oct 17, 2016

asaunier commented Oct 17, 2016

asaunier commented Oct 17, 2016 • edited Loading

tsauerwein commented Oct 17, 2016 • edited Loading

asaunier commented Oct 17, 2016

tsauerwein commented Oct 17, 2016

asaunier commented Oct 17, 2016 • edited Loading

asaunier commented Oct 19, 2016

asaunier commented Oct 19, 2016

asaunier commented Oct 19, 2016

tsauerwein commented Oct 20, 2016

asaunier commented Oct 20, 2016

tsauerwein commented Oct 21, 2016

ginold commented Oct 21, 2016

tsauerwein left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsauerwein commented Oct 21, 2016

asaunier commented Oct 21, 2016 • edited Loading

ginold commented Oct 21, 2016

asaunier commented Oct 12, 2016 •

edited by ginold

Loading

tsauerwein commented Oct 14, 2016 •

edited

Loading

ginold commented Oct 17, 2016 •

edited

Loading

asaunier commented Oct 17, 2016 •

edited

Loading

tsauerwein commented Oct 17, 2016 •

edited

Loading

asaunier commented Oct 17, 2016 •

edited

Loading

asaunier commented Oct 21, 2016 •

edited

Loading