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

Add the ability to link remote user profiles #5659

Merged
merged 1 commit into from Feb 13, 2015

Conversation

@Zauberstuhl
Copy link
Contributor

commented Feb 12, 2015

related to diaspora/jsxc#77

@Zauberstuhl Zauberstuhl changed the title Add the ability to link to remote user profile Add the ability to link remote user profiles Feb 12, 2015

id: params[:id] || params[:person_id],
username: username
})
end

This comment has been minimized.

Copy link
@jhass

jhass Feb 12, 2015

Member

While we have no Ruby styleguide yet, please at least decide on a hash form to use in a single diff hunk ;)

get '/u/:username' => 'people#show', :as => 'user_profile'
get '/u/:username/profile_photo' => 'users#user_photo'
get '/u/:username' => 'people#show', :as => 'user_profile', :constraints => { :username => /[^\/]+/ }
get '/u/:username/profile_photo' => 'users#user_photo', :constraints => { :username => /[^\/]+/ }

This comment has been minimized.

Copy link
@jhass

jhass Feb 12, 2015

Member

I'm not quite following on why you added those tbh.

This comment has been minimized.

Copy link
@Zauberstuhl

Zauberstuhl Feb 13, 2015

Author Contributor

http://guides.rubyonrails.org/routing.html#specifying-constraints

By default the :id parameter doesn't accept dots - this is because the dot is used as a separator for formatted routes. If you need to use a dot within an :id add a constraint which overrides this - for example id: /[^/]+/ allows anything except a slash.

This comment has been minimized.

Copy link
@Flaburgan

Flaburgan Feb 13, 2015

Member

Do we accept dots in username? I don't think so.

This comment has been minimized.

Copy link
@svbergerem

svbergerem Feb 13, 2015

Member

@Flaburgan No, but in the diaspora handle. (including @pod.tld)

@jhass

This comment has been minimized.

Copy link
Member

commented Feb 12, 2015

Should have a controller spec for the added functionality.

@Zauberstuhl

This comment has been minimized.

Copy link
Contributor Author

commented Feb 13, 2015

done

@jhass

This comment has been minimized.

Copy link
Member

commented Feb 13, 2015

Looking good, rebase into a single commit please.

@jhass jhass added this to the next-major milestone Feb 13, 2015

jhass added a commit that referenced this pull request Feb 13, 2015

Merge pull request #5659 from Zauberstuhl/link_to_remote_profile
Add the ability to link remote user profiles

@jhass jhass merged commit 84cfaa1 into diaspora:develop Feb 13, 2015

1 of 2 checks passed

continuous-integration/travis-ci The Travis CI build failed
Details
hound Hound has reviewed the changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.