Add keyboard shortcut to move to file permalink `y`
What does this MR do?
When viewing a file, pressing y will change the URL to a permalink that includes the full commit sha
http://localhost:3000/gitlab-org/gitlab-ce/blob/master/.eslintignore ->
http://localhost:3000/gitlab-org/gitlab-ce/blob/8ab94120ee0a87c7b1158ebafea101e3952ec758/.eslintignore
Previously, pressing y would copy the commit sha to the clipboard, 8ab94120ee0a87c7b1158ebafea101e3952ec758. This shortcut was not documented.
- MR: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4496
- Issue: https://gitlab.com/gitlab-org/gitlab-ce/issues/14470
Are there points in the code the reviewer needs to double check?
-
How do I get tree_joinin scope for use in the rspec tests? (using it overFile.joinbecause that is what the haml uses)
Why was this MR needed?
Screenshots (if relevant)
Does this MR meet the acceptance criteria?
-
Changelog entry added -
Documentation created/updated - Tests
-
Added for this feature/bug -
All builds are passing
-
-
Conform by the style guides -
Branch has no merge conflicts with master(if it does - rebase it please) -
Squashed related commits together -
EE MR
What are the relevant issue numbers?
Closes #8082 (closed)
Merge request reports
Activity
marked the task Squashed related commits together as completed
added 1 commit
- 00bbbce4 - Add keyboard shortcut to move to file permalink
added 1 commit
- 7168b444 - Add keyboard shortcut to move to file permalink
assigned to @iamphill
marked the task Conform by the style guides as completed
assigned to @MadLittleMods
added 1 commit
- 6ee82dc4 - Add keyboard shortcut to move to file permalink
@iamphill Updated and builds passing. Still curious about this issue but it's ok to move through https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8799#note_22090530
assigned to @iamphill
assigned to @MadLittleMods
added 191 commits
-
6ee82dc4...4468104f - 190 commits from branch
master - 67df6b0e - Add keyboard shortcut to move to file permalink
-
6ee82dc4...4468104f - 190 commits from branch
assigned to @iamphill
@MadLittleMods a couple comments for you
@MadLittleMods also, can you please get the ruby part to be reviewed by a backend endboss?
assigned to @MadLittleMods
added 498 commits
-
67df6b0e...572fb0be - 497 commits from branch
master - ea779c5b - Add keyboard shortcut to move to file permalink
-
67df6b0e...572fb0be - 497 commits from branch
added 1 commit
- 587059ee - Add keyboard shortcut to move to file permalink
assigned to @rspeicher
@rspeicher Please review the ruby part. Just some tests and perhaps the haml
Edited by username-removed-89286310 "http://#{page.server.host}:#{page.server.port}#{path}" 11 end 12 13 def visit_blob(fragment = "") 14 project.team << [user, :master] 15 login_as user 16 @path = project.repository.ls_files(project.repository.root_ref)[0] 17 @sha = project.repository.commit.sha 18 visit "#{namespace_project_blob_path(project.namespace, project, tree_join('master', @path))}#{"##{fragment}" unless fragment.empty?}" 19 end 20 21 describe 'pressing "y"' do 22 it 'redirects to permalink with commit sha' do 23 visit_blob() 24 find('body').native.send_key('y') 25 expect(page).to have_current_path(get_absolute_url(namespace_project_blob_path(project.namespace, project, tree_join(@sha, @path))), url: true) Very close to what I need.
namespace_project_blob_url(...)seems to use capybaradefault_host, which giveshttp://www.example.com/[...]instead of what the page is served athttp://127.0.0.1:62299/Edited by username-removed-892863@MadLittleMods Ah, fair enough. I think that port is even semi-random as well.
What we really care about is that the URL changes
masterto the canonical SHA. So maybe we can just doexpect(page.current_path).to end_with(...)(current_pathmight not be the right method, but you get the idea).page.current_pathdoesn't have the #fragment on the end which we want to make sure is maintained in the second testEdited by username-removed-892863
@MadLittleMods Couple notes, thanks!
assigned to @MadLittleMods
added 1 commit
- d757d931 - Add keyboard shortcut to move to file permalink
assigned to @rspeicher
added 1 commit
- ca3b9cae - Add keyboard shortcut to move to file permalink
added 1 commit
- 53f9116a - Add keyboard shortcut to move to file permalink
Thanks for all the testing tips @rspeicher
get_absolute_urlis still necessary to compare the full URL aspage.current_pathandpage_current_urlhave issues, see https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8799#note_22872788Edited by username-removed-892863added 1 commit
- 0ac4f007 - Add keyboard shortcut to move to file permalink
@MadLittleMods just a note
assigned to @MadLittleMods
added 1 commit
- cff1f12a - Add keyboard shortcut to move to file permalink
excellent @MadLittleMods LGTM! Is the ruby part ready?
@MadLittleMods @alfredo1 I found one more problem with the spec, but otherwise the Ruby LGTM.
@MadLittleMods Please remove the
putsline and make sure builds passes then assign it to me. I think the previous build passed with warning if so you probably have to create a MR with this changes to EE.Edited by username-removed-408881assigned to @MadLittleMods
added 1 commit
- 584b86da - Add keyboard shortcut to move to file permalink
mentioned in commit ff4950b7
changed milestone to %8.17
@MadLittleMods merged. Thanks!
