kevinburke / doony

UI Improvements for Jenkins
http://doony.org
Other
970 stars 116 forks source link

path to fix "build now" problems #91

Open mvk opened 8 years ago

mvk commented 8 years ago

NOTE: this is 1 of 2 issues causing these bugs #86 and #73

A regex used in several isJob*() methods does: return path.match(/^\/job\/.*?\//) !== null;, or similar. This regex assumes the value of window.location.pathname on a job page would always start with /job. This is untrue for various setups, esp. when jenkins is behind a reverse proxy of nginx/apachehttpd/etc., so the value of window.location.pathname can be /ci/job, or even nested: /apps/ci/job. failing to detect we're on a job page results in missing "Build Now" button. I'm suggesting this regex: return path.match(/^\/(.*\/)?job\/.*?\//), it allows the above paths, but probably could use some finer-tuning.

kevinburke commented 8 years ago

sounds good! Want to submit a PR

On Wednesday, January 20, 2016, Max Kovgan notifications@github.com wrote:

NOTE: this is 1 of 2 issues causing these bugs #86 https://github.com/kevinburke/doony/issues/86 and #73 https://github.com/kevinburke/doony/issues/73

A regex used in several isJob() methods does: return path.match(/^\/job\/.?\//) !== null;, or similar. This regex assumes the value of window.location.pathname on a job page would always start with /job. This is untrue for various setups, esp. when jenkins is behind a reverse proxy of nginx/apachehttpd/etc., so the value of window.location.pathname can be /ci/job, or even nested: /apps/ci/job. failing to detect we're on a job page results in missing "Build Now" button. I'm suggesting this regex: return path.match(/^\/(.\/)?job\/.?\//), it allows the above paths, but probably could use some finer-tuning.

— Reply to this email directly or view it on GitHub https://github.com/kevinburke/doony/issues/91.

Kevin Burke 925.271.7005 | kev.inburke.com

kevinburke commented 8 years ago

The compile process for this project is kind of a nightmare just make the change and I can do the rest

On Wednesday, January 20, 2016, Kevin Burke kev@inburke.com wrote:

sounds good! Want to submit a PR

On Wednesday, January 20, 2016, Max Kovgan <notifications@github.com javascript:_e(%7B%7D,'cvml','notifications@github.com');> wrote:

NOTE: this is 1 of 2 issues causing these bugs #86 https://github.com/kevinburke/doony/issues/86 and #73 https://github.com/kevinburke/doony/issues/73

A regex used in several isJob() methods does: return path.match(/^\/job\/.?\//) !== null;, or similar. This regex assumes the value of window.location.pathname on a job page would always start with /job. This is untrue for various setups, esp. when jenkins is behind a reverse proxy of nginx/apachehttpd/etc., so the value of window.location.pathname can be /ci/job, or even nested: /apps/ci/job. failing to detect we're on a job page results in missing "Build Now" button. I'm suggesting this regex: return path.match(/^\/(.\/)?job\/.?\//), it allows the above paths, but probably could use some finer-tuning.

— Reply to this email directly or view it on GitHub https://github.com/kevinburke/doony/issues/91.

Kevin Burke 925.271.7005 | kev.inburke.com

Kevin Burke 925.271.7005 | kev.inburke.com

mvk commented 8 years ago

@kevinburke I think I can run make, Actually this patch should be added after the 2nd one, b/c until the 2nd one is constructing good request - this one will expose the button which doesn't work.