Github: Unable to view reviews on PR

Created on 9 Oct 2019 · 8Comments · Source: atom/github

Prerequisites

[x] Put an X between the brackets on this line if you have done all of the following:
- Reproduced the problem in Safe Mode: https://flight-manual.atom.io/hacking-atom/sections/debugging/#using-safe-mode
- Followed all applicable steps in the debugging guide: https://flight-manual.atom.io/hacking-atom/sections/debugging/
- Checked the FAQs on the message board for common solutions: https://discuss.atom.io/c/faq
- Checked that your issue isn't already filed: https://github.com/issues?utf8=✓&q=is%3Aissue+user%3Aatom
- Checked that there is not already an Atom package that provides the described functionality: https://atom.io/packages

Description

Unable to view reviews on PR.

Steps to Reproduce

Edit: This are woefully incorrect

fork a repo
make a change and open pr on original repo not fork
have someone review this pr with a comment in the code
make a change which outdates the comment
try to open PR reviews from that PR.

Expected behavior:

See reviews

Actual behavior:

Uncaught (in promise) TypeError: Cannot read property 'diffToFilePosition' of undefined
    at ReviewsView.getTranslatedPosition (C:\Users\Wade\Documents\GitHub\github/reviews-view.js:631:40)
    at ReviewsView.commentThread (C:\Users\Wade\Documents\GitHub\github/reviews-view.js:389:45)
    at Array.map (<anonymous>)
    at ReviewsView.renderReviewCommentThreads (C:\Users\Wade\Documents\GitHub\github/reviews-view.js:359:30)
    at ReviewsView.render (C:\Users\Wade\Documents\GitHub\github/reviews-view.js:143:17)
    at Mh (C:\Users\Wade\Documents\GitHub\github\node_modules\react-dom\cjs\react-dom.production.min.js:173:192)
    at Lh (C:\Users\Wade\Documents\GitHub\github\node_modules\react-dom\cjs\react-dom.production.min.js:172:282)
    at mj (C:\Users\Wade\Documents\GitHub\github\node_modules\react-dom\cjs\react-dom.production.min.js:247:361)
    at jj (C:\Users\Wade\Documents\GitHub\github\node_modules\react-dom\cjs\react-dom.production.min.js:216:164)
    at Z (C:\Users\Wade\Documents\GitHub\github\node_modules\react-dom\cjs\react-dom.production.min.js:210:471)
    at $i (C:\Users\Wade\Documents\GitHub\github\node_modules\react-dom\cjs\react-dom.production.min.js:206:372)
    at C:\Users\Wade\Documents\GitHub\github\node_modules\react-dom\cjs\react-dom.production.min.js:114:115
    at exports.unstable_runWithPriority (C:\Users\Wade\Documents\GitHub\github\node_modules\react-dom\node_modules\scheduler\cjs\scheduler.production.min.js:20:134)
    at vf (C:\Users\Wade\Documents\GitHub\github\node_modules\react-dom\cjs\react-dom.production.min.js:113:325)
    at yf (C:\Users\Wade\Documents\GitHub\github\node_modules\react-dom\cjs\react-dom.production.min.js:114:61)
    at O (C:\Users\Wade\Documents\GitHub\github\node_modules\react-dom\cjs\react-dom.production.min.js:113:476)
    at eg (C:\Users\Wade\Documents\GitHub\github\node_modules\react-dom\cjs\react-dom.production.min.js:204:235)
    at Object.enqueueSetState (C:\Users\Wade\Documents\GitHub\github\node_modules\react-dom\cjs\react-dom.production.min.js:127:237)
    at ObserveModel.D.setState (C:\Users\Wade\Documents\GitHub\github\node_modules\react\cjs\react.production.min.js:13:231)
    at ModelObserver.ObserveModel._defineProperty [as didUpdate] (C:\Users\Wade\Documents\GitHub\github/observe-model.js:49:12)
    at ModelObserver._refreshModelData (C:\Users\Wade\Documents\GitHub\github/model-observer.js:52:14)

Reproduces how often:

100% with this setup.

Versions

Atom : 1.42.0-nightly8
Electron: 4.2.7
Chrome : 69.0.3497.128
Node : 10.11.0

OS: Windows 10

Additional Information

bug

Source

wadethestealth

All 8 comments

Thanks for reporting - I was unable to reproduce on Windows 10 with 1.42.0-nightly8 but I tested with a toy repository. Were you able to reproduce with multiple repositories/pull requests? Is there a public repository + pull request that causes the error that we can test with?

rsese on 10 Oct 2019

Repository I was using is nteract/hydrogen and the PR that seems to cause this is nteract/hydrogen#1715. It may also be worth noting that is the branch I have checked out too.

Maybe I am assuming the steps incorrectly.

wadethestealth on 10 Oct 2019

Also you may need to note I am using the master branch of atom/github. The error also only kicks in when I click "see reviews" the rendered page works just fine.

wadethestealth on 10 Oct 2019

here is the line its breaking on. This reason is because rootComment.path is lib\config.js, but in translations the key is lib/config.js this causes the function unable to find my translations.

wadethestealth on 10 Oct 2019

A quick solution to my problems is to change line 630 to const translationsForFile = translations.get(path.normalize(rootComment.path));

wadethestealth on 11 Oct 2019

👍1

@wadethestealth Good sleuthing. Want to add a test case and make a PR?