allows bounty URLs to be blocked #5430
Conversation
owocki
requested review from
danlipert,
octavioamu and
thelostone-mc
as code owners
|
Thanks! Seems like this is missing server-side blacklisting, though. Client side validation is the same as no validation, in practice. |
fair! let me see what i can cludge into the backend quickly. |
Codecov Report
@@ Coverage Diff @@
## master #5430 +/- ##
==========================================
+ Coverage 30.22% 30.24% +0.01%
==========================================
Files 247 247
Lines 21036 21048 +12
Branches 3032 3034 +2
==========================================
+ Hits 6358 6365 +7
- Misses 14399 14404 +5
Partials 279 279
Continue to review full report at Codecov.
|
| @@ -0,0 +1,23 @@ | |||
| # Generated by Django 2.2.3 on 2019-10-30 21:03 | |||
There was a problem hiding this comment.
Seems to me this doesn't belong here
|
any feedback @danlipert ? gonna merge soon |
app/assets/v2/js/pages/new_bounty.js
Outdated
| @@ -462,6 +462,15 @@ $('#sync-issue').on('click', function(event) { | |||
| }); | |||
|
|
|||
| $('#issueURL').focusout(function() { | |||
| for (var i = 0; i <= document.blocked_urls.length; i++) { | |||
| var this_url_filter = document.blocked_urls[i]; | |||
app/assets/v2/js/pages/new_bounty.js
Outdated
| @@ -462,6 +462,15 @@ $('#sync-issue').on('click', function(event) { | |||
| }); | |||
|
|
|||
| $('#issueURL').focusout(function() { | |||
| for (var i = 0; i <= document.blocked_urls.length; i++) { | |||
There was a problem hiding this comment.
do we wanna add a document.blocked_urls check ?
There was a problem hiding this comment.
let / const
i increments; why would i want to make it a constant?
document.blocked_urls
no.. itll always exist
There was a problem hiding this comment.
i increments; why would i want to make it a constant?
aka let to avoid hoisting it :P
There was a problem hiding this comment.
TIL about hoisting.. committing a few now
app/assets/v2/js/pages/new_bounty.js
Outdated
| var this_url_filter = document.blocked_urls[i]; | ||
|
|
||
| if ($('input[name=issueURL]').val().toLowerCase().indexOf(this_url_filter.toLowerCase()) != -1) { | ||
| alert('this repo is not bountyable at the request of the maintainer.'); |
|
@owocki are you going to add the backend check in this branch? |
|
@danlipert wont be able to do so before todays release.. i'd say, if we think this looks ok on the frontend, lets get it into todays release -- and i will add a note to create a hard-backend-exception as a hotfix or in next weeks release. |
|
@danlipert @thelostone-mc fc8db60 addresses ur feedabcks |
Description
allows bounty URLs to be blocked
Refers/Fixes
see #product-feedback ; this is a user request
Testing
tested locally