Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update avatar URL for a user + refactoring #15

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions app/model/user/User.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ public class User extends EntityBase
private String fullName;

private String emailAddress;

private String avatarURL;

@Override
protected void setId(final Long id)
Expand All @@ -41,6 +43,7 @@ public String getUserName()
public void setUserName(String userName)
{
this.userName = userName;
setAvatarURL(userName);
}

public String getFullName()
Expand All @@ -62,4 +65,15 @@ public void setEmailAddress(String emailAddress)
{
this.emailAddress = emailAddress;
}

public String getAvatarURL()
{
return avatarURL;
}

public String setAvatarURL(String userName)
{
/* Below URL is of the Crucible ( codereview ) server */
this.avatarURL = "http://srv-ind-scrat.vanenburg.com:8060/avatar/" + userName;
Copy link

@astellingwerf astellingwerf May 18, 2016

Choose a reason for hiding this comment

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

Shouldn't the ?s=number be part of this code as well? That'd make it trivial to switch to another avatar source (like Jira).

Copy link
Author

Choose a reason for hiding this comment

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

The required size of the avatar (requested with the ?s=number GET parameter) is dependent on the location where it is used. Currently three times 32, and one time 40.
If you really think it is valuable to make that split right now, maybe 'avatarURLSmall' and avatarURLMedium' or something like that can be introduced for that.

}
}
2 changes: 1 addition & 1 deletion public/partials/task/task.item.dashboard.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
{{task.summary}}
<span class="pull-right smaller-font">
Updated: {{task.updateDate | date:'MM/dd/yy HH:mm'}}<BR>
<img class="pull-right" tooltip="{{task.assignee.fullName}}" ng-show="task.assignee" ng-src="http://srv-ind-scrat:8060/avatar/{{task.assignee.userName}}?s=32"/>
<img class="pull-right" tooltip="{{task.assignee.fullName}}" ng-show="task.assignee" ng-src="{{task.assignee.avatarURL}}?s=32"/>
</span>
<BR>
<span class="smaller-font" ng-show="task.details" ng-bind-html="task.details">
Expand Down
2 changes: 1 addition & 1 deletion public/partials/task/task.item.panel.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
{{task.summary}}
<span class="pull-right" style="font-size: 12px;" >
{{task.updateDate | date:'MM/dd/yy @ h:mma'}}
<img ng-show="task.assignee" ng-src="http://srv-ind-scrat:8060/avatar/{{task.assignee.userName}}?s=32"/>
<img ng-show="task.assignee" ng-src="{{task.assignee.avatarURL}}?s=32"/>
</span>
<BR>
</div>
2 changes: 1 addition & 1 deletion public/partials/user/detail.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<fieldset>
<legend>Edit user {{user.userName}}</legend>
<div class="pull-right">
<img ng-src="http://srv-ind-scrat:8060/avatar/{{user.userName}}?s=40" />
<img ng-src="{{user.avatarURL}}?s=40" />
Copy link
Owner

Choose a reason for hiding this comment

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

As the username is filled in this form, the {{user.userName}} is updated dynamically.
So while changing the user name, you can directly see the results for the avatar.
In the pull request, that changes as the url is only set in the backend, changes are only visible after save.

Do you have a specific use case for changing the way of setting the avatar link?
In case you want to support other location in future, it is better to put the ?s= also in saved url and give the img tags a size. In that case the avatars can be requested with s=40 in all cases, can it can be resized to 32x32.
Using ?=s was introduced to request the images as small as possible (network load) and still have good pictures.

In case you put a size on the image tags and get the avatar from a different location, then it will still be displayed with the right size.

</div>
<div class="form-group" ng-class="{'has-error': myForm.userName.$invalid}">
<label for="userName" class="col-sm-2 control-label">Username</label>
Expand Down
2 changes: 1 addition & 1 deletion public/partials/user/list.html
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
<td>{{user.userName}}</td>
<td>{{user.fullName}}</td>
<td>{{user.emailAddress}}</td>
<td><img ng-src="http://srv-ind-scrat:8060/avatar/{{user.userName}}?s=32"/></td>
<td><img ng-src="{{user.avatarURL}}?s=32"/></td>
</tr>
</tbody>
</table>
Expand Down