GitLab FOSS merge requestshttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests2017-10-03T23:23:07Zhttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/14561Only copy old/new code when selecting left/right side of parallel diff2017-10-03T23:23:07ZDouwe MaanOnly copy old/new code when selecting left/right side of parallel diff## Old
![Old](https://cl.ly/152t3o120T15/Screen%20Recording%202017-09-28%20at%2004.53%20PM.gif)
## New
![New](https://cl.ly/242U0Q1s2y2t/Screen%20Recording%202017-09-28%20at%2004.51%20PM.gif)## Old
![Old](https://cl.ly/152t3o120T15/Screen%20Recording%202017-09-28%20at%2004.53%20PM.gif)
## New
![New](https://cl.ly/242U0Q1s2y2t/Screen%20Recording%202017-09-28%20at%2004.51%20PM.gif)10.1Phil Hughesme@iamphill.comPhil Hughesme@iamphill.comhttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/14225Make the labels in the Compare form less confusing2018-12-14T00:03:53Zusername-removed-128633Make the labels in the Compare form less confusing## What does this MR do?
* Improve the descriptive text
* Rename "from" to "Target" and "to" to "Source"
* Swap "Target" and "Source" to have the same order as in MRs
* Reworded "Switch base of comparison" to "Swap versions"
## ...## What does this MR do?
* Improve the descriptive text
* Rename "from" to "Target" and "to" to "Source"
* Swap "Target" and "Source" to have the same order as in MRs
* Reworded "Switch base of comparison" to "Swap versions"
## Are there points in the code the reviewer needs to double check?
I didn't update the params name (i.e. `from` and `to`) as they're used in a lot of different context and they're not shown to the end-user in the compare page.
## Why was this MR needed?
Because the page was confusing.
## Screenshots (if relevant)
| Before | After |
| ------ | ----- |
| ![before0](/uploads/4496935afa572e068dfa9c7e482a5bf0/before0.png) | ![after0](/uploads/bec7283e0f3fd18abbc39916316d2e90/after0.png) |
| ![before1](/uploads/687b4bf64cbf21df8d6ca665b892a00c/before1.png) | ![after1](/uploads/fca9a6048c360b70e9ba7abd81da02d1/after1.png) |
| ![before2](/uploads/600bacb3896467641b0679603d713e5d/before2.png) | ![after2](/uploads/79207d11c545dea24583f7dc747df19e/after2.png) |
| ![before3](/uploads/ee6a165a41a67772e698058099ff2e92/before3.png) | ![after3](/uploads/247a3dc9d530c5cf3c27d9e9c0645e19/after3.png) |
## Does this MR meet the acceptance criteria?
- [x] [Changelog entry](https://docs.gitlab.com/ce/development/changelog.html) added, if necessary
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- Review
- [x] Has been reviewed by UX
- [ ] Has been reviewed by Frontend
- [ ] Has been reviewed by Backend
- [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
## What are the relevant issue numbers?
Closes #2133110.1Phil Hughesme@iamphill.comPhil Hughesme@iamphill.comhttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/14093WIP: 36220 Adds file diff for each commit in a file's commit history page.2017-09-18T18:38:47Zusername-removed-1517404WIP: 36220 Adds file diff for each commit in a file's commit history page.## What does this MR do?
This adds a file-diff view for each commit in a file's commit history.
## Why was this MR needed?
To close/fulfill #36220
## Does this MR meet the acceptance criteria?
- [ ] [Changelog entry](https://d...## What does this MR do?
This adds a file-diff view for each commit in a file's commit history.
## Why was this MR needed?
To close/fulfill #36220
## Does this MR meet the acceptance criteria?
- [ ] [Changelog entry](https://docs.gitlab.com/ce/development/changelog.html) added, if necessary
- [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [ ] API support added
- [ ] Tests added for this feature/bug
- Review
- [ ] Has been reviewed by UX
- [ ] Has been reviewed by Frontend
- [ ] Has been reviewed by Backend
- [ ] Has been reviewed by Database
- [ ] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [ ] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [ ] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
## What are the relevant issue numbers?
Closes #36220username-removed-1517404username-removed-1517404https://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/14061Commenting on image diffs2017-11-22T00:10:11ZFelipe ArturCommenting on image diffs## What does this MR do?
This MR primarily adds the ability to comment on image diffs. This includes merge requests and commit diffs. When you click on an image in the image diff, you can create discussions. Each time you click on the i...## What does this MR do?
This MR primarily adds the ability to comment on image diffs. This includes merge requests and commit diffs. When you click on an image in the image diff, you can create discussions. Each time you click on the image (on a different area/coordinate), you will create a new discussion.
In addition, we also did the following on the frontend:
- Consolidated the number of places where we initialize gl.ImageFile()
- Converted `btn-comment` into a `mixin`, so that it can be reused for image diffs
- Fixed an edge case on gl.ImageFile whereby occasionally the images wouldn't trigger the 2-up view because the image was loaded before gl.ImageFile was invoked
## I don't see the collapse icon SVG anymore. How do I get my local system setup?
1. Clone https://gitlab.com/gitlab-org/gitlab-svgs locally
1. Checkout branch `add-collapse-icon` (at least until https://gitlab.com/gitlab-org/gitlab-svgs/merge_requests/5 gets merged)
1. Run `yarn install` (first time only)
1. Run `yarn run svg`
1. Replace `dist/icons.svg` (gitlab-svg repo) with `app/assets/images/icon.svg` (this repo)
## Are there points in the code the reviewer needs to double check?
Make sure everything looks :100:
## Why was this MR needed?
~Deliverable
## Screenshots (if relevant)
### Architecture
| Diffs | MR Discussion tab |
|---|---|
|![Screen_Shot_2017-10-02_at_4.08.09_PM](/uploads/626f38e030419df0bed7da8d926870d3/Screen_Shot_2017-10-02_at_4.08.09_PM.png)|![Screen_Shot_2017-10-02_at_4.08.13_PM](/uploads/fb45e8d1c4792de4f8ec2265606116f5/Screen_Shot_2017-10-02_at_4.08.13_PM.png)|
| commenting on gif in MR diff tab | commenting on gif in discussion tab |
|---|---|
|![2017-10-02_16.14.22](/uploads/b2488f34e473ddf77f15331028b35a3f/2017-10-02_16.14.22.gif)|![2017-10-02_16.16.38](/uploads/3e50221ab776a52142f6a434bbc99031/2017-10-02_16.16.38.gif)|
| Replaced image in MR diff tab | Replaced image in discussion tab |
|---|---|
|![2017-10-02_16.18.29](/uploads/3a2dae163481fb09b20d7e0c7f280334/2017-10-02_16.18.29.gif)|![2017-10-02_16.19.01](/uploads/348d755c44d528efb9cfa4f227a4b055/2017-10-02_16.19.01.gif)|
| commenting on commit diff | commit diff discussion in MR |
|---|---|
|![Screen_Shot_2017-10-02_at_4.22.26_PM](/uploads/914a89c6955047058beaa05ced50fafd/Screen_Shot_2017-10-02_at_4.22.26_PM.png)|![Screen_Shot_2017-10-02_at_4.23.21_PM](/uploads/92e655cbae203cd4caff46fe3f83ed26/Screen_Shot_2017-10-02_at_4.23.21_PM.png)|
## Does this MR meet the acceptance criteria?
- [x] [Changelog entry](https://docs.gitlab.com/ee/development/changelog.html) added, if necessary
- [ ] [Documentation created/updated](https://docs.gitlab.com/ee/development/doc_styleguide.html) (Will create in another MR)
- [x] Tests added for this feature/bug
- Review
- [x] Has been reviewed by UX (@dimitrieh)
- [ ] Has been reviewed by Frontend
- [ ] Has been reviewed by Backend
- [ ] Has been reviewed by Database
- [x] Conform by the [merge request performance guides](https://docs.gitlab.com/ee/development/merge_request_performance_guidelines.html)
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
## What are the relevant issue numbers?
closes #35873, #38559
<hr>
#### Remaining Todos:
Remaining items:
- [x] Disable commenting when not logged in (@brycepj)
- [x] jagged borders (@brycepj)
- [x] update badge counters on avatar when new comment in an existing discussion is added (@ClemMakesApps)
- [x] update badge counters on avatar when comment is edited (@ClemMakesApps)
- [x] update badge counters on image when comment is deleted (@ClemMakesApps)
- [x] toggle collapse of image diff discussion (@brycepj)
- [x] fix jump to unresolved discussion (@ClemMakesApps)
- [x] enable image diff on multiple image views (@ClemMakesApps)
- [x] highlight when linked (fix multiple highlights) (@ClemMakesApps)
- [x] Add image diff to discussion tab (@ClemMakesApps)
- [x] Fix z-index of collapsed discussion badge overlap with jagged border (@ClemMakesApps)
- [x] Unable to comment on images if the image loads collapsed by default
- [x] Commenting doesn't work properly when it is replaced view but the 2-up, swipe, onion skin modes don't kick in (related to https://gitlab.com/gitlab-org/gitlab-ce/issues/38559)
- [x] Ensure Flash message is shown on comment error (@brycepj)
- [x] Make image badges and avatar badges round circles (https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14061#note_41928050) (6cbd2ff @brycepj)
- [x] Update blue dot in image diff discussion (on discussion tab) to use the diff comment image (https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14061#note_41928050)
- [x] Add blue commenting dot to author of image diff discussion (on discussion tab) (https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14061#note_41928050) (@ClemMakesApps)
- [x] no blue halo around pointers in images, see https://gitlab.com/gitlab-org/gitlab-ce/issues/35873#note_41934515 (9d68089 @brycepj)
- [x] After an image diff discussion is resolved (and you refresh the MR in the discussion tab), and expand the resolved discussion, the comments do not appear (@ClemMakesApps)
- [x] Center align collapse/badge number buttons in image diff discussions (diff tab)
- [x] Increase collapse toggle button size in image diff discussions to match badge number button size (diff tab)
- [x] Check unbindEvents method to make sure binded events are removed
- [x] Add jagged border between last discussion and new discussion comment form
- [x] Get polling working with image diff (@brycepj)
- [x] Review scss
- [x] Comment form bug https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14061#note_42080525MakesApps
- [x] Remove duplicate from `/javascripts/diff_notes/icons/collapse_icon.svg` (created ~"technical debt" )
- [x] Glitch every time we click changes tab (https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14061#note_42116310) (@ClemMakesApps)
- [x] Focus halo is still present (https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14061#note_42116310) (@ClemMakesApps)
- [x] When collapsing a discussion after having focussed on the editor to create a reply when it was open.. the editor stays in view.. either in small mode or big mode (https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14061#note_42116310)
- [x] Initialize replaced image view modes on discussion-tab (@ClemMakesApps)
- [x] Center align the diff collapse icon
- [x] Adding new discussion on diff tab doesn't automatically add avatar badge anymore
#### Tests
Karma Tests
===
- [x] badge_helper
- [x] comment_indicator_helper
- [x] dom_helper
- [x] utils_helper
- [x] image_badge
- [x] image_diff
- [x] init_discussion_tab
- [x] replaced_image_diff
- [x] view_types
- [x] image_utility
Rspec Tests
===
- [x] Clicking on image creates image diff discussion
- [x] Resolved diff discussions load collapsed
- [x] Adding a comment to an existing diff discussion shows avatar badge
- [x] Polling a comment shows avatar badge
- [x] Image diff view modes show discussion badges (on discussion tab)
- [x] Image diff view modes show discussion badges (on diffs tab)
- [x] Image diff displays on discussion tab
- [x] Image diff displays on diffs tab
- [x] Image diff displays on commit diff
#### Pushed to next iteration
1. avatar tooltips when hovering over badge on the image diff
1. miniature version of the chat, when collapsing a discussion
1. clicking on image badges should scroll to first note in the discussion (both discussion and changes tab, in a sensible way which tries to keep the image in context with the first comment in the discussion)
1. add shadows to badge and cursor so that it's easier to visualize the depth
1. create image diff discussions on the older image to another iteration
1. clicking on the edge of the image in swipe view mode causes the comment indicator to get clipped (https://tppr.me/oAbGP)
1. 2-up view isn't always placed side by side. We should change it so that it is. I think it is displayed on top of each other when the image width is over a certain size
#### Known ~"UX debt" (acceptable to move forward know that these exist, will create individual issues for each of them before this gets merged and approved by @dimitrieh)
1) when you delete the first note of a discussion, the avatar badge does not get populated for the new first note (until you refresh the page)
2) when you edit the first note of a discussion, the avatar badge does not get displayed after the editing is saved (until you refresh the page)
3) If you do scenario 1 and then you switch to the other tab (discussion if you were on diff tab, diff tab if you were on discussion) and assuming those two tabs are already loaded, the avatar badge will not get populated (until you refresh the page)
4) If you do scenario 2 and then you switch to the other tab (discussion if you were on diff tab, diff tab if you were on discussion) and assuming those two tabs are already loaded, the avatar badge will not get populated (until you refresh the page)
5) Toggling nav bar and sidebar does not update the image badge positions to match their positions even though the image size changes10.1Jacob SchatzJacob Schatzhttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/13882WIP: Add functionality to collapse outdated diff comments regardless of discu...2017-09-05T15:15:14Zusername-removed-726483WIP: Add functionality to collapse outdated diff comments regardless of discussion resolution## What does this MR do?
Add repository toggle for automatically resolving outdated diff discussions.
## Are there points in the code the reviewer needs to double check?
## Why was this MR needed?
In an MR, GitLab will only colla...## What does this MR do?
Add repository toggle for automatically resolving outdated diff discussions.
## Are there points in the code the reviewer needs to double check?
## Why was this MR needed?
In an MR, GitLab will only collapse comments on diffs that have been marked as resolved unlike on GitHub where they are automatically collapsed when the diff becomes outdated. There is no setting in repositories to toggle this functionality. This makes sense for when developers address the comments through code in a follow-up commit thus outdating the comment(s) on the previous commit. GitLab once behaved like this but the feature was removed.
## Screenshots (if relevant)
![automatically_resolve_outdated_diff_discussions](/uploads/2ef4724d8b9486023aeea93a1394901c/automatically_resolve_outdated_diff_discussions.png)
## Does this MR meet the acceptance criteria?
- [x] [Changelog entry](https://docs.gitlab.com/ce/development/changelog.html) added, if necessary
- [x] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [x] API support added
- [x] Tests added for this feature/bug
- Review
- [ ] Has been reviewed by UX
- [ ] Has been reviewed by Frontend
- [ ] Has been reviewed by Backend
- [ ] Has been reviewed by Database
- [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [ ] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
## What are the relevant issue numbers?
Closes #36994username-removed-443319username-removed-443319https://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/13156Resolve "Comment appears in a wrong place after changing diff view to inline"2017-08-08T09:09:50Zusername-removed-443319Resolve "Comment appears in a wrong place after changing diff view to inline"## What does this MR do?
Fixes diff commenting results when you changed your diff view in this request.
## Are there points in the code the reviewer needs to double check?
I thought about a feature spec, but that seems very heav...## What does this MR do?
Fixes diff commenting results when you changed your diff view in this request.
## Are there points in the code the reviewer needs to double check?
I thought about a feature spec, but that seems very heavy for this case.
## Why was this MR needed?
It was a (probably very old) bug!
## Does this MR meet the acceptance criteria?
- [x] [Changelog entry](https://docs.gitlab.com/ce/development/changelog.html) added, if necessary
- Tests
- [x] Added for this feature/bug
- [ ] All builds are passing
- Review
- [ ] Has been reviewed by Backend
- [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if it does - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
## What are the relevant issue numbers?
Closes #35695.9.5Grzegorz BizonGrzegorz Bizonhttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/12332Fix linking to line number on parallel diff creating empty discussion box2017-06-23T11:58:22Zusername-removed-892863contact@ericeastwood.comFix linking to line number on parallel diff creating empty discussion box## What does this MR do?
EE MR, https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2207
Fix linking to line number on parallel diff creating empty discussion box when there are discussions on the same line.
## Are there points in ...## What does this MR do?
EE MR, https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/2207
Fix linking to line number on parallel diff creating empty discussion box when there are discussions on the same line.
## Are there points in the code the reviewer needs to double check?
- Using actual inline/parallel diff ajax responses in specs
Reproduction:
1. Create a MR with some changes
1. Switch to the parallel "Side-by-side" view, `?view=parallel`
1. Create a note on the diff and resolve it
1. Click the line number that has the discussion on it. Your URL should look similar to this `/diffs?view=parallel#3f58c61b62d64910afb975d31ceed28eb21c34c1_28_35`
1. Copy and visit the URL
1. Notice, only the line is highlighted (no empty gray discussion box is shown)
## Why was this MR needed?
Linking to line number on parallel diff when there are discussions on the same line, created an empty discussion box.
![](http://i.imgur.com/rvfA1CK.png)
This regressed in 9.2 from https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11327/diffs#192369e4731a70b7fe3155cc69bbbd55b33c7932_281_287
## Does this MR meet the acceptance criteria?
- [x] [Changelog entry](https://docs.gitlab.com/ce/development/changelog.html) added, if necessary
- Tests
- [x] Added for this feature/bug
- [x] All builds are passing
- [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if it does - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
## What are the relevant issue numbers?
Closes #340109.3Jacob SchatzJacob Schatzhttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/12297Fix pressing up-arrow on a MR "Changes" diff discussion edits your last note2017-06-21T02:58:46Zusername-removed-892863contact@ericeastwood.comFix pressing up-arrow on a MR "Changes" diff discussion edits your last note## What does this MR do?
Fix pressing up-arrow on a MR "Changes" diff discussion edits your last note.
<img src="http://imgur.com/4zfXsC1.gif" width="50%">
## Are there points in the code the reviewer needs to double check?
I suspect...## What does this MR do?
Fix pressing up-arrow on a MR "Changes" diff discussion edits your last note.
<img src="http://imgur.com/4zfXsC1.gif" width="50%">
## Are there points in the code the reviewer needs to double check?
I suspect this regressed in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11173/diffs#f4bb984ec495c5cb58516785c62978f5209c39b4_179_178 where we changed the selector to be more specific on where it looks.
## Why was this MR needed?
Pressing up-arrow on a MR "Changes" diff discussion didn't do anything.
## Does this MR meet the acceptance criteria?
- Tests
- [x] Added for this feature/bug
- [x] All builds are passing
- [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if it does - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
## What are the relevant issue numbers?
Closes #338689.3Jacob SchatzJacob Schatzhttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/12048Only look up diff size limit flags once per request2017-06-20T14:31:22Zusername-removed-443319Only look up diff size limit flags once per requestWithout this, we'd query for the feature flag for every diff file - which can be a lot on a single page! Even worse, we'd do it several times for each diff file, as these methods are assumed to be free!
Once we've run the experiment wit...Without this, we'd query for the feature flag for every diff file - which can be a lot on a single page! Even worse, we'd do it several times for each diff file, as these methods are assumed to be free!
Once we've run the experiment with the feature flags, I'll remove them anyway. For now, caching in `RequestStore` should remove the performance regression I introduced.9.3Douwe MaanDouwe Maanhttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/11875Increase diff limits to 100 KB for collapse and 200 KB overall2017-10-02T13:48:10Zusername-removed-443319Increase diff limits to 100 KB for collapse and 200 KB overallCloses #31983.Closes #31983.9.3Douwe MaanDouwe Maanhttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/11702Fix note header author and time ago wrapping in parallel diff2017-06-08T15:44:47Zusername-removed-892863contact@ericeastwood.comFix note header author and time ago wrapping in parallel diff## What does this MR do?
- Fix note header author and time ago wrapping in parallel diff
See subsequent MR https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11701 where we apply all the same responsive note styles to the parallel...## What does this MR do?
- Fix note header author and time ago wrapping in parallel diff
See subsequent MR https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11701 where we apply all the same responsive note styles to the parallel diff notes.
## Are there points in the code the reviewer needs to double check?
## Why was this MR needed?
. | Screenshot
--- | ---
Before | ![](https://i.imgur.com/LxuRabK.png)
After | ![](https://i.imgur.com/ngbpv2C.png)
## Does this MR meet the acceptance criteria?
- Tests
- [x] All builds are passing
- [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if it does - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
## What are the relevant issue numbers?
Closes #328019.2Annabel GrayAnnabel Grayhttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/11700Fix error thrown with missing note fragment in DOM2017-06-12T08:03:35Zusername-removed-892863contact@ericeastwood.comFix error thrown with missing note fragment in DOM## What does this MR do?
- Fix error thrown when loading the diff tab that doesn't have the note associated with the fragment hash in the URL.
## Are there points in the code the reviewer needs to double check?
Reproduction:
1. Vis...## What does this MR do?
- Fix error thrown when loading the diff tab that doesn't have the note associated with the fragment hash in the URL.
## Are there points in the code the reviewer needs to double check?
Reproduction:
1. Visit a MR with a note hash that doesn't exist, `/namespace/project/merge_requests/x/diffs#note_1234`
1. Notice the error throw when the diff tab loads
## Why was this MR needed?
- An error is thrown when loading the diff tab that doesn't have the note associated with the fragment hash in the URL.
## Screenshots (if relevant)
## Does this MR meet the acceptance criteria?
- Tests
- [x] Added for this feature/bug
- [x] All builds are passing
- [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if it does - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
## What are the relevant issue numbers?
Closes #328889.2username-removed-408677username-removed-408677https://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/11609Fix jQuery data attribute caching issue causing expanding issues2017-05-23T16:41:15Zusername-removed-892863contact@ericeastwood.comFix jQuery data attribute caching issue causing expanding issues## What does this MR do?
- Use vanilla `dataset` to retrieve `data-line-type` attribute
- The jQuery flavor was returning an empty string and failing our logic below. From debugging the jQuery code, seems to be a internal jQuery ca...## What does this MR do?
- Use vanilla `dataset` to retrieve `data-line-type` attribute
- The jQuery flavor was returning an empty string and failing our logic below. From debugging the jQuery code, seems to be a internal jQuery caching issue `this.cache( owner )`. The attribute is in place before the command is run.
- Here is same spot in `notes.js` before all the changes this release. It still boiled down to `$(e.currentTarget || e.target).data('lineType')` previously so it is unclear how we haven't run into this before, https://gitlab.com/gitlab-org/gitlab-ce/blob/0fc1e04f4c15e9d5adbb4e47c82d3eebf8b48110/app/assets/javascripts/notes.js#L826
## Are there points in the code the reviewer needs to double check?
## Why was this MR needed?
In parallel diff view, find resolve/collapsed diff comment. Page-load and try to expand the diff.
---
Command | Result
--- | ---
$link.data('lineType') | ''
$link.data('line-type') | ''
$link.attr('data-line-type') | 'new'
$link[0].dataset.lineType | 'new'
## Screenshots (if relevant)
## Does this MR meet the acceptance criteria?
- Tests
- [ ] Added for this feature/bug
- [x] All builds are passing
- [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if it does - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
## What are the relevant issue numbers?
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/327059.2Phil Hughesme@iamphill.comPhil Hughesme@iamphill.comhttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/11584Add system note with link to diff comparison when MR discussion becomes outdated2017-06-06T16:49:00ZDouwe MaanAdd system note with link to diff comparison when MR discussion becomes outdatedFixes https://gitlab.com/gitlab-org/gitlab-ce/issues/30058.
#### Active diff discussion
![Screen_Shot_2017-05-21_at_15.27.07](/uploads/5c8a053723867ee1dff55abecc9d6120/Screen_Shot_2017-05-21_at_15.27.07.png)
#### Outdated diff discuss...Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/30058.
#### Active diff discussion
![Screen_Shot_2017-05-21_at_15.27.07](/uploads/5c8a053723867ee1dff55abecc9d6120/Screen_Shot_2017-05-21_at_15.27.07.png)
#### Outdated diff discussion
![Screen_Shot_2017-05-21_at_15.30.47](/uploads/b32e93240a47b3d2ba701d1bd0bfac46/Screen_Shot_2017-05-21_at_15.30.47.png)
#### "version 1 of the diff link" leads us to diff comparison with the changed line highlighted
![Screen_Shot_2017-05-21_at_15.31.05](/uploads/132668cd419269ec66dfd693a75ef3ed/Screen_Shot_2017-05-21_at_15.31.05.png)
/cc @smcgivern @victorwu9.3Grzegorz BizonGrzegorz Bizonhttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/11458Fix linking to unresolved/expanded diff note2017-09-05T14:34:26Zusername-removed-892863contact@ericeastwood.comFix linking to unresolved/expanded diff note## What does this MR do?
- When linking to a note on the diffs tab that is unresolved and expanded by default, we will no longer collapse it accidentally.
## Are there points in the code the reviewer needs to double check?
## Why wa...## What does this MR do?
- When linking to a note on the diffs tab that is unresolved and expanded by default, we will no longer collapse it accidentally.
## Are there points in the code the reviewer needs to double check?
## Why was this MR needed?
- When linking to a note on the diffs tab that is unresolved and expanded by default, we accidentally collapse it.
- Bug introduced in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11327
## Does this MR meet the acceptance criteria?
- Tests
- [x] Added for this feature/bug
- [x] All builds are passing
- [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if it does - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
## What are the relevant issue numbers?
Closes #324249.2Phil Hughesme@iamphill.comPhil Hughesme@iamphill.comhttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/11327Fix linking to resolved note in diff2017-09-05T14:34:26Zusername-removed-892863contact@ericeastwood.comFix linking to resolved note in diff## What does this MR do?
EE MR: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1901
- Fix linking to resolved note in diff
- Will scroll to visible linked note
## Are there points in the code the reviewer needs to ...## What does this MR do?
EE MR: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1901
- Fix linking to resolved note in diff
- Will scroll to visible linked note
## Are there points in the code the reviewer needs to double check?
## Why was this MR needed?
- Linking to a resolved note on the diff tab doesn't show the note, ex, `http://localhost:3000/gitlab-org/gitlab-ce/merge_requests/8/diffs#note_995`
## Does this MR meet the acceptance criteria?
- Tests
- [x] All builds are passing
- [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if it does - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
## What are the relevant issue numbers?
Closes #321259.2https://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/10868Fix diffs with edit-forking needs2017-04-25T18:28:49Zusername-removed-892863contact@ericeastwood.comFix diffs with edit-forking needs## What does this MR do?
EE MR: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1705/
- Fix MR diffs where the user is required to fork in order to "Edit" a file
MR https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10602 ...## What does this MR do?
EE MR: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1705/
- Fix MR diffs where the user is required to fork in order to "Edit" a file
MR https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10602 caused https://gitlab.com/gitlab-org/gitlab-ce/issues/31276
## Are there points in the code the reviewer needs to double check?
- When you don't have permissions to edit directly and need a fork, when you already have a fork, and try to edit a file, it will ask you to fork again and error out, https://gitlab.com/gitlab-org/gitlab-ce/issues/31281
- This bug was also present before this new fork/cancel confirmation, [`blob_helper` -> `edit_blob_link`](https://gitlab.com/gitlab-org/gitlab-ce/blob/c5a9d73ad8a141166d871e551027208014a281c0/app/helpers/blob_helper.rb#L25)
## Why was this MR needed?
- When viewing a MR from fork (from another use other than author), the page would 500, https://gitlab.com/gitlab-org/gitlab-ce/issues/31276
## Does this MR meet the acceptance criteria?
- Tests
- [x] Added for this feature/bug
- [x] All builds are passing
- [x] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if it does - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
## What are the relevant issue numbers?
Closes #31276Douwe MaanDouwe Maanhttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/10761Diff comment button debounce2017-04-25T19:03:32ZLuke "Jared" BennettDiff comment button debounce## What does this MR do?
~~Debounce the show, delay the hide and return early as possible for performance~~
Debounces the rendering of the comment button when `mouseover` a diff line.
Delays the hiding of the comment button when...## What does this MR do?
~~Debounce the show, delay the hide and return early as possible for performance~~
Debounces the rendering of the comment button when `mouseover` a diff line.
Delays the hiding of the comment button when `mouseleave` a diff line.
Returns as early as possible during the rendering/showing/hiding of the comment button to reduce wasted computation.
## Are there points in the code the reviewer needs to double check?
## Why was this MR needed?
## Screenshots (if relevant)
## Does this MR meet the acceptance criteria?
- [ ] [Changelog entry](https://docs.gitlab.com/ce/development/changelog.html) added, if necessary
- [ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)
- [ ] API support added
- Tests
- [ ] Added for this feature/bug
- [ ] All builds are passing
- [ ] Conform by the [merge request performance guides](http://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
- [ ] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [ ] Branch has no merge conflicts with `master` (if it does - rebase it please)
- [ ] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)
## What are the relevant issue numbers?
Related to https://gitlab.com/gitlab-org/gitlab-ce/issues/304049.2Jacob SchatzJacob Schatzhttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/10733Added break-word wrap2017-04-18T07:46:14ZLuke "Jared" BennettAdded break-word wrap## What does this MR do?
Fixes a rare case of diff overflow
## Screenshots (if relevant)
### Before
![2017-04-17_11.13.46](/uploads/7d2b290f30ca83dcf1bb592ea1b243c1/2017-04-17_11.13.46.gif)
### After
![Screen_Shot_2017-...## What does this MR do?
Fixes a rare case of diff overflow
## Screenshots (if relevant)
### Before
![2017-04-17_11.13.46](/uploads/7d2b290f30ca83dcf1bb592ea1b243c1/2017-04-17_11.13.46.gif)
### After
![Screen_Shot_2017-04-17_at_11.15.56](/uploads/d18e62d1a0e63336b9b07907548316e0/Screen_Shot_2017-04-17_at_11.15.56.png)
## What are the relevant issue numbers?
Closes #309869.1Annabel GrayAnnabel Grayhttps://staging.gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/10355WIP: Download patch with code comments for unresolved discussions2017-07-28T15:09:54ZDouwe MaanWIP: Download patch with code comments for unresolved discussions/cc @rspeicher @smcgivern
Example patch:
```patch
From 80ed295019941ebaf3377c7202c9120ea932425e Mon Sep 17 00:00:00 2001
From: Administrator <admin@example.com>
Date: Thu, 30 Mar 2017 17:18:32 -0600
Subject: [PATCH] FIXME: Add code co.../cc @rspeicher @smcgivern
Example patch:
```patch
From 80ed295019941ebaf3377c7202c9120ea932425e Mon Sep 17 00:00:00 2001
From: Administrator <admin@example.com>
Date: Thu, 30 Mar 2017 17:18:32 -0600
Subject: [PATCH] FIXME: Add code comments for unresolved discussions from
gitlab-org/gitlab-ce!8
All unresolved non-outdated diff discussions from this merge request are
added in the relevant places as code comments with a 'FIXME' prefix.
This commit was automatically generated by GitLab. Be sure to remove it
from your branch before the merge request is merged.
---
app/models/user.rb | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/app/models/user.rb b/app/models/user.rb
index 2db35dc2be..5636d1ae11 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -94,6 +94,11 @@ class User < ActiveRecord::Base
# the user is destroyed. If the user owns any issues during deletion, this
# should be treated as an exceptional condition.
has_many :issues, dependent: :restrict_with_exception, foreign_key: :author_id
+ # FIXME: Administrator (@root) started a discussion on the previous line:
+ # Should we do the same for notes?
+ #
+ # FIXME: Administrator (@root) commented:
+ # I guess we can do that later
#
# Validations
@@ -334,6 +339,8 @@ class User < ActiveRecord::Base
ghost_user ||
begin
users = Enumerator.new do |y|
+ # FIXME: Administrator (@root) started a discussion on the previous line:
+ # This is pretty hard to understand
n = nil
loop do
user = User.new(
--
2.11.0
```
To do:
- [ ] Cache commit
- [ ] Consider including discussions on deleted lines
- [ ] Consider including outdated diff discussions
- [ ] Consider including non-diff discussions
- [ ] Comments in more languages
- [ ] Improve text in modal
- [ ] Specs
- [ ] DocsDouwe MaanDouwe Maan