-
-
Notifications
You must be signed in to change notification settings - Fork 771
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
project_types (formerly work schemes) #1098
Conversation
app/assets/v2/js/pages/dashboard.js
Outdated
'tech_stack' | ||
'tech_stack', | ||
'work_scheme', | ||
'application_scheme', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unexpected trailing comma. (comma-dangle)
app/assets/v2/js/pages/dashboard.js
Outdated
@@ -452,8 +454,8 @@ var refreshBounties = function(event) { | |||
result.action = result['url']; | |||
result['title'] = result['title'] ? result['title'] : result['github_url']; | |||
|
|||
|
|||
result['p'] = ((result['experience_level'] ? result['experience_level'] : 'Unknown Experience Level') + ' • '); | |||
var work_scheme = ucwords(result['work_scheme']) + ' • '; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected blank line after variable declarations. (newline-after-var)
app/assets/v2/js/pages/new_bounty.js
Outdated
@@ -137,6 +137,10 @@ $(document).ready(function() { | |||
githubUsername: metadata.githubUsername, | |||
address: '' // Fill this in later | |||
}, | |||
schemes: { | |||
work_scheme: data.work_scheme, | |||
application_scheme: data.application_scheme, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unexpected trailing comma. (comma-dangle)
app/assets/v2/js/shared.js
Outdated
@@ -95,6 +95,12 @@ var sanitizeAPIResults = function(results) { | |||
return results; | |||
}; | |||
|
|||
function ucwords (str) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unexpected space before function parentheses. (space-before-function-paren)
app/assets/v2/js/shared.js
Outdated
@@ -95,6 +95,12 @@ var sanitizeAPIResults = function(results) { | |||
return results; | |||
}; | |||
|
|||
function ucwords (str) { | |||
return (str + '').replace(/^([a-z])|\s+([a-z])/g, function ($1) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected indentation of 2 spaces but found 4. (indent)
Unexpected space before function parentheses. (space-before-function-paren)
app/assets/v2/js/shared.js
Outdated
@@ -95,6 +95,12 @@ var sanitizeAPIResults = function(results) { | |||
return results; | |||
}; | |||
|
|||
function ucwords (str) { | |||
return (str + '').replace(/^([a-z])|\s+([a-z])/g, function ($1) { | |||
return $1.toUpperCase(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected indentation of 4 spaces but found 8. (indent)
app/assets/v2/js/shared.js
Outdated
function ucwords (str) { | ||
return (str + '').replace(/^([a-z])|\s+([a-z])/g, function ($1) { | ||
return $1.toUpperCase(); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected indentation of 2 spaces but found 4. (indent)
app/dashboard/router.py
Outdated
@@ -118,7 +119,8 @@ def get_queryset(self): | |||
|
|||
# filtering | |||
for key in ['raw_data', 'experience_level', 'project_length', 'bounty_type', 'bounty_owner_address', | |||
'idx_status', 'network', 'bounty_owner_github_username', 'standard_bounties_id']: | |||
'idx_status', 'network', 'bounty_owner_github_username', 'standard_bounties_id', | |||
'application_scheme', 'work_scheme' ]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
E202 whitespace before ']'
Codecov Report
@@ Coverage Diff @@
## master #1098 +/- ##
==========================================
- Coverage 30.64% 30.28% -0.37%
==========================================
Files 124 125 +1
Lines 8790 8999 +209
Branches 1141 1155 +14
==========================================
+ Hits 2694 2725 +31
- Misses 5988 6166 +178
Partials 108 108
Continue to review full report at Codecov.
|
@@ -514,7 +514,8 @@ var do_actions = function(result) { | |||
pull_interest_list(result['pk'], function(is_interested) { | |||
|
|||
// which actions should we show? | |||
var show_start_stop_work = is_still_on_happy_path; | |||
var should_block_from_starting_work = !is_interested && result['work_scheme'] == 'traditional' && (result['status'] == 'started' || result['status'] == 'submitted') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon. (semi)
Can we rename Application Scheme ---> Approval Process? Do we want to encourage work contests? |
theres a whole long convo about this at gitcoinco/gitcoinco#9 |
Enforcing limit that a person can not start ticket if the person is already doing other ticket. |
}, | ||
'work_scheme': function(key, val, result) { | ||
return [ 'work_scheme', result.work_scheme ]; | ||
return [ 'bounty_owner_name', result.bounty_owner_name ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unreachable code. (no-unreachable)
schemes through start work flow
@@ -421,7 +429,8 @@ var show_interest_modal = function() { | |||
var self = this; | |||
|
|||
setTimeout(function() { | |||
$.get('/interest/modal?redirect=' + window.location.pathname, function(newHTML) { | |||
var url = '/interest/modal?redirect=' + window.location.pathname + "&pk=" + document.result['pk'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected blank line after variable declarations. (newline-after-var)
Strings must use singlequote. (quotes)
Missing semicolon. (semi)
app/dashboard/notifications.py
Outdated
@@ -392,21 +392,39 @@ def build_github_notification(bounty, event_name, profile_pairs=None): | |||
"* Questions? Checkout <a href='https://gitcoin.co/help'>Gitcoin Help</a> or <a href='https://gitcoin.co/slack'>Gitcoin Slack</a>\n * " \ | |||
f"${amount_open_work} more funded OSS Work available on the [Gitcoin Issue Explorer](https://gitcoin.co/explorer)\n" | |||
elif event_name == 'work_started': | |||
interested = bounty.interested.all() | |||
interestd_plural = "s" if interested.count() != 0 else "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
F841 local variable 'interestd_plural' is assigned to but never used
app/dashboard/notifications.py
Outdated
msg = f"{status_header}__Work has been started__.\n{profiles} has committed to working on this project to be " \ | ||
f"completed {from_now}.\n\n" | ||
started_work = bounty.interested.filter(pending=False).all() | ||
pending_approval = bounty.interested.filter(pending=True).all() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
F841 local variable 'pending_approval' is assigned to but never used
app/dashboard/notifications.py
Outdated
|
||
issue_message = interest.issue_message.strip() | ||
if issue_message: | ||
msg += f"{bounty_owner_clear} they have the following comments/questions for you:\n\n```{issue_message}```" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
E501 line too long (123 > 120 characters)
app/dashboard/views.py
Outdated
@@ -190,14 +205,14 @@ def new_interest(request, bounty_id): | |||
|
|||
if profile.has_been_removed_by_staff(): | |||
return JsonResponse({ | |||
'error': _('Because a staff member has had to remove you from a bounty in the past, you are unable to start more work at this time. Please contact support.'), | |||
'error': _('Because a staff member has had to remove you from a bounty in the past, you are unable to start more work at this time. Please leave a message on slack if you feel this message is in error.'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
E501 line too long (216 > 120 characters)
app/dashboard/views.py
Outdated
if is_funder or is_staff: | ||
interests = bounty.interested.filter(pending=True, profile__handle=worker) | ||
if not interests.exists(): | ||
messages.warning(request, _('This worker does not exist or is not in a pending state. Please check your link and try again.')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
E501 line too long (142 > 120 characters)
app/dashboard/views.py
Outdated
interest.pending = False | ||
interest.save() | ||
|
||
# TODO: send an email when |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
W291 trailing whitespace
app/dashboard/views.py
Outdated
maybe_market_to_slack(bounty, 'worker_approved') | ||
maybe_market_to_user_slack(bounty, 'worker_approved') | ||
maybe_market_to_twitter(bounty, 'worker_approved') | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
W293 blank line contains whitespace
app/dashboard/views.py
Outdated
maybe_market_to_twitter(bounty, 'worker_approved') | ||
|
||
|
||
else: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
E303 too many blank lines (2)
mocked up github issue for starting work in the new work schemes paradigm http://bits.owocki.com/17161E2M1x18/Screen%20Shot%202018-05-20%20at%202.31.01%20PM.png |
example gitcoinbot comment thread owocki/pytrader#96 |
Initial thoughts: The copy: Maybe we could restructure the comment to have less repetitive copy with something like: Option 1Issue Status: 1. Open 2. Started 3. Submitted 4. Done Work has been started. @owocki These users each claimed they can complete the work within 12 months from now. Please review their questions below:
Option 2Issue Status: 1. Open 2. Started 3. Submitted 4. Done Work has been started. These users each claimed they can complete the work within 12 months from now. Please review their questions below: Start WorkAwaiting Approval
Approved
Option 3Issue Status: 1. Open 2. Started 3. Submitted 4. Done Work has been started. These users each claimed they can complete the work within 12 months from now:
Questions:@owocki Please review the following questions:
|
app/dashboard/helpers.py
Outdated
@@ -414,7 +414,7 @@ def create_new_bounty(old_bounties, bounty_payload, bounty_details, bounty_id): | |||
# Currently we are only considering the latest fulfillment. Std bounties supports multiple. | |||
# If any of the fulfillments have been accepted, the bounty is now accepted and complete. | |||
accepted = any([fulfillment.get('accepted') for fulfillment in fulfillments]) | |||
|
|||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
W293 blank line contains whitespace
app/dashboard/views.py
Outdated
@@ -178,6 +186,13 @@ def new_interest(request, bounty_id): | |||
except Bounty.DoesNotExist: | |||
raise Http404 | |||
|
|||
is_too_many_already_started = bounty.work_scheme == 'traditional' and bounty.interested.filter(pending=False).count() > 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
E501 line too long (125 > 120 characters)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could do:
is_too_many_already_started = bounty.work_scheme == 'traditional' and bounty.interested.filter(pending=False).exists()
to avoid the COUNT()
.
Also, can we add Bounty.SCHEME_TYPES
and the individual scheme types, so: bounty.work_scheme == 'traditional'
would become: bounty.work_scheme == Bounty.SCHEME_TRADITIONAL
or something like that.
Bounty.work_scheme
would be modified to include choices=SCHEME_TYPES
@mbeacom i like option number 1 |
Not to bloat up the work schemes, but I wonder if it might makes sense to allow for a progressive input under Work Scheme: Traditional so that the funder can tag a contributor that they're reserving the issue for. Example: Reserved for @owocki |
i like the idea.. can we v2 it? |
@owocki @mbeacom could we consider option 2 over 1. Issue Status: 1. Open 2. Started 3. Submitted 4. Done Work has been started. @owocki These users each claimed they can complete the work within 12 months from now. Please review their questions below: Awaiting Approval (Only funder can approve / reject worker) Approved
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -136,6 +138,15 @@ var callbacks = { | |||
'bounty_owner_name': function(key, val, result) { | |||
return [ 'bounty_owner_name', result.bounty_owner_name ]; | |||
}, | |||
'permission_type': function(key, val, result) { | |||
if(val == 'approval'){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected space(s) after "if". (keyword-spacing)
Missing space before opening brace. (space-before-blocks)
app/app/urls.py
Outdated
@@ -238,6 +238,14 @@ | |||
url(r'^_administration/email/start_work_expired$', retail.emails.start_work_expired, name='start_work_expired'), | |||
re_path(r'^_administration/email/gdpr_reconsent$', retail.emails.gdpr_reconsent, name='gdpr_reconsent'), | |||
url(r'^_administration/email/new_tip/resend$', retail.emails.resend_new_tip, name='resend_new_tip'), | |||
url(r'^_administration/process_accesscode_request/(.*)$', tdi.views.process_accesscode_request, name='process_accesscode_request'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make fix-yapf
or make-fix
@@ -133,7 +138,10 @@ def gh_login(request): | |||
|
|||
def get_interest_modal(request): | |||
|
|||
bounty = Bounty.objects.get(pk=request.GET.get("pk")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we catch if request.GET.get('pk')
is not present?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should always be present per the JS tho?
|
||
<h1>{% trans "24 Hrs to Approve" %}</h1> | ||
<div style="text-align: left! important"> | ||
<p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we switch this to two-space indentation for all of the new html templates throughout?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let me see how to auto lint this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
app/dashboard/notifications.py
Outdated
msg = f"{status_header}__Workers have applied to start work__.\n\n" | ||
|
||
msg += f"\nThese users each claimed they can complete the work by {from_now}. " \ | ||
"Please review their questions below:\n\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
E127 continuation line over-indented for visual indent
…into kevin/application_schemes
app/dashboard/models.py
Outdated
@@ -934,12 +983,21 @@ class Interest(models.Model): | |||
"""Define relationship for profiles expressing interest on a bounty.""" | |||
|
|||
profile = models.ForeignKey('dashboard.Profile', related_name='interested', on_delete=models.CASCADE) | |||
created = models.DateTimeField(auto_now_add=True, blank=True, null=True) | |||
created = models.DateTimeField(auto_now_add=True, blank=True, null=True) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
W291 trailing whitespace
…into kevin/application_schemes
This reverts commit e8e3041.
… the denomination dropdown
…into kevin/application_schemes
app/assets/v2/js/pages/new_bounty.js
Outdated
$('.js-select2').each(function() { | ||
$(this).select2(); | ||
}); | ||
//removes tooltip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected space or tab after '//' in comment. (spaced-comment)
app/assets/v2/js/pages/new_bounty.js
Outdated
$('.js-select2').each(function() { | ||
$(this).select2(); | ||
}); | ||
//removes tooltip | ||
$('select').on('change', function (evt) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unexpected space before function parentheses. (space-before-function-paren)
app/assets/v2/js/pages/new_bounty.js
Outdated
$('.js-select2').each(function() { | ||
$(this).select2(); | ||
}); | ||
//removes tooltip | ||
$('select').on('change', function (evt) { | ||
$('.select2-selection__rendered').removeAttr('title'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected indentation of 4 spaces but found 6. (indent)
app/assets/v2/js/pages/new_bounty.js
Outdated
$('select').on('change', function (evt) { | ||
$('.select2-selection__rendered').removeAttr('title'); | ||
}); | ||
//removes search field in all but the 'denomination' dropdown |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected space or tab after '//' in comment. (spaced-comment)
app/assets/v2/js/pages/new_bounty.js
Outdated
$('.select2-selection__rendered').removeAttr('title'); | ||
}); | ||
//removes search field in all but the 'denomination' dropdown | ||
$('.select2-container').click(function(){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing space before opening brace. (space-before-blocks)
app/assets/v2/js/pages/new_bounty.js
Outdated
}); | ||
//removes search field in all but the 'denomination' dropdown | ||
$('.select2-container').click(function(){ | ||
$('.select2-container .select2-search__field').remove(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected indentation of 4 spaces but found 6. (indent)
app/assets/v2/js/pages/new_bounty.js
Outdated
$('.select2-container').click(function(){ | ||
$('.select2-container .select2-search__field').remove(); | ||
}); | ||
//denomination field |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected space or tab after '//' in comment. (spaced-comment)
Description
Support for
work schemes
and
see also - alisa's email copy doc
Still TODO
to release
Checklist
Affected core subsystem(s)
all of it
Testing
tested it
Refers/Fixes
#973