-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Redirect /dataset/[id] -> /dataset/[name] etc #4316
Comments
Apart from commented out block, LGTM. The only thing really is occasionally there are cases where the user is redirected to /dataset/ID (don't ask me for an example off the top of my head, but I'm definitely not imagining it) - it's going to mean another HTTP(S) request for the redirect until we track those down. Probably not block-worthy though. |
davidread
pushed a commit
that referenced
this issue
Jul 9, 2018
wardi
added a commit
that referenced
this issue
Jul 13, 2018
[#4316] Redirect /dataset/[id] -> /dataset/[name] etc
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Redirects:
Specifying these objects by id is hidden functionality really. There are no links to it. I've added it for two reasons:
As a developer it's occasionally I know a dataset's id and want to look at it, and it is useful to have it instead display on the normal url with the name in it. Why would you want the current functionality and keep it as the weird id url anyway? It's functionality I've wanted on many occasions over the years - maybe it's just me!
In Activity Stream the links to old versions of datasets/groups/organizations are currently given using the 'name' of those objects as they were at the time of the change. Obviously there are ways that these objects can change name, and in this case the link gets broken. So with my changes in PR Revision UI removed & activity stream improved #3972 I'm keen to change the link to be the id of the objects, so we avoid the broken link problem, and we don't add a whole bunch of joins on these queries that are potentially already quite big.
The text was updated successfully, but these errors were encountered: