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

"View issues on Github" link on Search result page does not work #1730

Closed
tkbremnes opened this issue Aug 29, 2017 · 8 comments
Closed

"View issues on Github" link on Search result page does not work #1730

tkbremnes opened this issue Aug 29, 2017 · 8 comments

Comments

@tkbremnes
Copy link

How to reproduce:

  • Search for any issue
  • Scroll down to the bottom of the page, and click the View issues on Github link.
  • Get sent to a Github 404 page

It appears that the template is not stamping out the URL correctly, leading to the nonsensical URL https://github.com/{{ config['ISSUES_REPO_URI'] }}/{{ number }}

Code line in question:

<a class="wc-Link wc-GithubLink" href="https://github.com/{{ config['ISSUES_REPO_URI'] }}/{{ number }}"><span class="wc-GithubLink-Icon"></span>View issue on Github</a>

@karlcow
Copy link
Member

karlcow commented Aug 29, 2017

  1. Go to https://webcompat.com/
  2. Click on the search button at the top
  3. Enter mozilla We get https://webcompat.com/issues?page=1&per_page=50&state=open&stage=all&sort=created&direction=desc&q=mozilla
  4. Indeed the bottom link is https://github.com/%7B%7B%20config['ISSUES_REPO_URI']%20%7D%7D/%7B%7B%20number%20%7D%7D

That's a bug. Thanks @tkbremnes

@karlcow
Copy link
Member

karlcow commented Aug 29, 2017

That was added by @tagawa in #572 And it is working for individual issues, but not for issues search.

g doesn't work either as it is redirecting simply to https://github.com/webcompat/web-bugs/issues and not the related search.

Two options here:

  • Remove entirely the feature from this template. Easy
  • OR Make it work for this template, link and g. But that could be slightly more complex to convert the search string into an equivalent search string for GitHub.

@tkbremnes did you need this feature? Or did you fall on it by accident?

@tkbremnes
Copy link
Author

Fell onto it by accident. Don't even know where the link should have taken me, so don't know what to expect.

@miketaylr
Copy link
Member

Indeed the bottom link is https://github.com/%7B%7B%20config['ISSUES_REPO_URI']%20%7D%7D/%7B%7B%20number%20%7D%7D

I possibly broke this when I messed around with templates recently.

I think it would be nice for this to work, but it will require a bit of work as @karlcow mentions. Perhaps a good way forward is to remove the link, and file a bug to add it back (with the proper webcompat -> github param translation).

@karlcow
Copy link
Member

karlcow commented Aug 30, 2017

→ http GET 'https://webcompat.com/issues?page=1&per_page=50&state=open&stage=all&sort=created&direction=desc&q=mozilla' | subl

To see the raw body which is sent to the browser. This part has been set correctly.

<main role="main" data-repo-path="webcompat/web-bugs/issues">

  </main>

Oh wait…

<p>
  <a class="wc-Link wc-GithubLink" href="https://github.com/webcompat/web-bugs/issues/"><span class="wc-GithubLink-Icon"></span>View issues on Github</a>
</p>
<p>
  <span class="wc-GithubLink-DesktopOnly">Shortcut: Press <b>g</b> on your keyboard to be taken to the GitHub view of this page.</span>
</p>

This is what is sent by the raw html response.
@miketaylr do you rewrite the content which is being sent how does it return to the value in the Jinja template.

Ah… indeed.

webcompat/templates/list-issue/issuelist-issue.jst
61:  <a class="wc-Link wc-GithubLink" href="https://github.com/{{ config['ISSUES_REPO_URI'] }}/{{ number }}"><span class="wc-GithubLink-Icon"></span>View issues on Github</a>

Does that mean the Jinja template is moot now? or at least not reflecting the current state of affairs.

ok let's try to cook up a patch…

Oh 💩 These are in a file I didn't touch with my commits. I don't think this should happen.

→ git commit -m 'Issue #1730 - Removes the link to GitHub view' .

> husky - npm run -s precommit
> husky - node v6.10.2

 ❯ Running tasks for *.js
   ✖ lint:JS
     →     at Parser.pp$3.parseFunctionExpression (/Users/karl/code/webcompat.com/node_modules/prettier/node_modules/babylon/lib/index.js:3725:17)
 ↓ Running tasks for *.css [skipped]
   → No staged files match *.css
🚫 lint:JS found some errors. Please fix them and try committing again.
'with' in strict mode (3:103)
  1 | this["wcTmpl"] = this["wcTmpl"] || {};
  2 | 
> 3 | this["wcTmpl"]["issue/aside.jst"] = function(obj) {obj || (obj = {});var __t, __p = '', __e = _.escape;with (obj) {__p += '<header class="wc-Issue-information-header wc-Issue-information-header--' +((__t = ( stateClass )) == null ? '' : __t) +'">\n    <div>' +((__t = ( issueState )) == null ? '' : __t) +'</div>\n    <div>#' +((__t = ( number )) == null ? '' : __t) +'</div>\n  </header>\n  <div class="wc-Issue-information-content">\n    <h1 class="wc-Issue-title js-Issue-title">\n      ' +((__t = ( title )) == null ? '' : __t) +'\n    </h1>\n  </div>\n  <div class="wc-Issue-information-content">\n    <div>Opened: ' +((__t = ( createdAt )) == null ? '' : __t) +'</div>\n    <div>\n      Reporter:\n      <span class="wc-Issue-reporter js-Issue-reporter">\n        <a class="wc-Link" href="https://github.com/' +((__t = ( reporter )) == null ? '' : __t) +'">' +((__t = ( reporter )) == null ? '' : __t) +'</a>\n      </span>\n    </div>\n    <div>Comments: ' +((__t = ( commentNumber )) == null ? '' : __t) +'</div>\n  </div>';}return __p};
    |                                                                                                        ^
  4 | 
  5 | this["wcTmpl"]["issue/issue-comment-list.jst"] = function(obj) {obj || (obj = {});var __t, __p = '', __e = _.escape;with (obj) {__p += '<div class="wc-Comment-body">\n    <div class="wc-Comment-wrapper">\n      <div class="wc-Comment-header">\n        <div class="wc-Comment-owner">\n          <div class="wc-Comment-owner-avatar">\n            <img src="' +((__t = ( avatarUrl )) == null ? '' : __t) +'" alt="' +((__t = ( commenter )) == null ? '' : __t) +'">\n          </div>\n          <div class="wc-Comment-owner-name js-Comment-owner">\n            <a class="wc-Link" href="https://github.com/' +((__t = ( commenter )) == null ? '' : __t) +'">' +((__t = ( commenter )) == null ? '' : __t) +'</a>\n          </div>\n        </div>\n        <div>\n          <a class="wc-Link" href="#' +((__t = ( commentLinkId )) == null ? '' : __t) +'">' +((__t = ( createdAt )) == null ? '' : __t) +'</a>\n        </div>\n      </div>\n      <div class="wc-Comment-content js-Comment-content">\n        <div class="wc-Markdown">\n          ' +((__t = ( body )) == null ? '' : __t) +'\n        </div>\n      </div>\n    </div>\n  </div>';}return __p};
  6 | 
SyntaxError: 'with' in strict mode (3:103)
  1 | this["wcTmpl"] = this["wcTmpl"] || {};
  2 | 
> 3 | this["wcTmpl"]["issue/aside.jst"] = function(obj) {obj || (obj = {});var __t, __p = '', __e = _.escape;with (obj) {__p += '<header class="wc-Issue-information-header wc-Issue-information-header--' +((__t = ( stateClass )) == null ? '' : __t) +'">\n    <div>' +((__t = ( issueState )) == null ? '' : __t) +'</div>\n    <div>#' +((__t = ( number )) == null ? '' : __t) +'</div>\n  </header>\n  <div class="wc-Issue-information-content">\n    <h1 class="wc-Issue-title js-Issue-title">\n      ' +((__t = ( title )) == null ? '' : __t) +'\n    </h1>\n  </div>\n  <div class="wc-Issue-information-content">\n    <div>Opened: ' +((__t = ( createdAt )) == null ? '' : __t) +'</div>\n    <div>\n      Reporter:\n      <span class="wc-Issue-reporter js-Issue-reporter">\n        <a class="wc-Link" href="https://github.com/' +((__t = ( reporter )) == null ? '' : __t) +'">' +((__t = ( reporter )) == null ? '' : __t) +'</a>\n      </span>\n    </div>\n    <div>Comments: ' +((__t = ( commentNumber )) == null ? '' : __t) +'</div>\n  </div>';}return __p};
    |                                                                                                        ^
  4 | 
  5 | this["wcTmpl"]["issue/issue-comment-list.jst"] = function(obj) {obj || (obj = {});var __t, __p = '', __e = _.escape;with (obj) {__p += '<div class="wc-Comment-body">\n    <div class="wc-Comment-wrapper">\n      <div class="wc-Comment-header">\n        <div class="wc-Comment-owner">\n          <div class="wc-Comment-owner-avatar">\n            <img src="' +((__t = ( avatarUrl )) == null ? '' : __t) +'" alt="' +((__t = ( commenter )) == null ? '' : __t) +'">\n          </div>\n          <div class="wc-Comment-owner-name js-Comment-owner">\n            <a class="wc-Link" href="https://github.com/' +((__t = ( commenter )) == null ? '' : __t) +'">' +((__t = ( commenter )) == null ? '' : __t) +'</a>\n          </div>\n        </div>\n        <div>\n          <a class="wc-Link" href="#' +((__t = ( commentLinkId )) == null ? '' : __t) +'">' +((__t = ( createdAt )) == null ? '' : __t) +'</a>\n        </div>\n      </div>\n      <div class="wc-Comment-content js-Comment-content">\n        <div class="wc-Markdown">\n          ' +((__t = ( body )) == null ? '' : __t) +'\n        </div>\n      </div>\n    </div>\n  </div>';}return __p};
  6 | 
    at Parser.pp$5.raise (/Users/karl/code/webcompat.com/node_modules/prettier/node_modules/babylon/lib/index.js:4429:13)
    at Parser.pp$1.parseWithStatement (/Users/karl/code/webcompat.com/node_modules/prettier/node_modules/babylon/lib/index.js:2162:31)
    at Parser.pp$1.parseStatement (/Users/karl/code/webcompat.com/node_modules/prettier/node_modules/babylon/lib/index.js:1844:19)
    at Parser.parseStatement (/Users/karl/code/webcompat.com/node_modules/prettier/node_modules/babylon/lib/index.js:5781:22)
    at Parser.pp$1.parseBlockBody (/Users/karl/code/webcompat.com/node_modules/prettier/node_modules/babylon/lib/index.js:2246:21)
    at Parser.pp$1.parseBlock (/Users/karl/code/webcompat.com/node_modules/prettier/node_modules/babylon/lib/index.js:2225:8)
    at Parser.pp$3.parseFunctionBody (/Users/karl/code/webcompat.com/node_modules/prettier/node_modules/babylon/lib/index.js:4197:22)
    at Parser.parseFunctionBody (/Users/karl/code/webcompat.com/node_modules/prettier/node_modules/babylon/lib/index.js:5768:20)
    at Parser.pp$1.parseFunction (/Users/karl/code/webcompat.com/node_modules/prettier/node_modules/babylon/lib/index.js:2363:8)
    at Parser.pp$3.parseFunctionExpression (/Users/karl/code/webcompat.com/node_modules/prettier/node_modules/babylon/lib/index.js:3725:17)



> husky - pre-commit hook failed (add --no-verify to bypass)
> husky - to debug, use 'npm run precommit'

huh

→ npm run precommit

> webcompat@ precommit /Users/karl/code/webcompat.com
> lint-staged

 ↓ Running tasks for *.js [skipped]
   → No staged files match *.js
 ↓ Running tasks for *.css [skipped]
   → No staged files match *.css

@karlcow
Copy link
Member

karlcow commented Aug 30, 2017

Oh wait

→ git status
On branch 1730/1
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

	modified:   webcompat/static/js/templates.js
	modified:   webcompat/templates/list-issue/issuelist-issue.jst

no changes added to commit (use "git add" and/or "git commit -a")

Raising eyebrows… webcompat/static/js/templates.js What The Heck?

checking .gitignore

env/
node_modules/
git_modules/
.DS_Store
config.py
fabfile.py
*.pyc
*.db
docs/build/
screenshots/
uploads/
.idea/
config/secrets.py
package-lock.json

# The data folder contains information that shouldn't live in version control.
data/*

# these are our 'dist' files
# checking them into the repo is silly
webcompat/static/js/webcompat.js
webcompat/static/js/diagnose.js
webcompat/static/js/issues.js
webcompat/static/js/issue-list.js
webcompat/static/js/user-activity.js
webcompat/static/js/cssfixme.js
webcompat/static/js/templates.js
webcompat/**/*.min.js
webcompat/**/*.min.css
webcompat/**/*.dev.css

#selenium server
*.jar
*.pid
nohup.out

karlcow added a commit to karlcow/webcompat.com that referenced this issue Aug 30, 2017
@karlcow
Copy link
Member

karlcow commented Aug 30, 2017

OK let's see

→ git rm --cached webcompat/static/js/templates.js
rm 'webcompat/static/js/templates.js'
→ git status
On branch 1730/1
Changes to be committed:
  (use "git reset HEAD <file>..." to unstage)

	deleted:    webcompat/static/js/templates.js

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

	modified:   webcompat/templates/list-issue/issuelist-issue.jst

let's commit it and back to business.

@karlcow
Copy link
Member

karlcow commented Aug 30, 2017

I fixed my woes with

webcompat = origin here.

git fetch webcompat
git reset --hard webcompat/master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants