WIP on jobs board #1102
WIP on jobs board #1102
Conversation
|
|
||
| var set_filter_header = function() { | ||
| var idxStatusEl = $('input[name=idx_status]:checked'); | ||
| var filter_status = idxStatusEl.attr('val-ui') ? idxStatusEl.attr('val-ui') : 'All'; |
There was a problem hiding this comment.
Expected blank line after variable declarations. (newline-after-var)
app/assets/v2/js/pages/jobs.js
Outdated
| var idxStatusEl = $('input[name=idx_status]:checked'); | ||
| var filter_status = idxStatusEl.attr('val-ui') ? idxStatusEl.attr('val-ui') : 'All'; | ||
| // TODO: See what all filters are to be displayed from designs | ||
| $('#filter').html("All"); |
There was a problem hiding this comment.
Strings must use singlequote. (quotes)
app/assets/v2/js/pages/jobs.js
Outdated
|
|
||
| var matchesEl = $('#matches'); | ||
| var | ||
| listingInfoEl = $('#listing-info'); |
There was a problem hiding this comment.
Expected indentation of 6 spaces but found 2. (indent)
| @@ -0,0 +1,3 @@ | |||
| from django.test import TestCase | |||
There was a problem hiding this comment.
F401 'django.test.TestCase' imported but unused
app/jobs/views.py
Outdated
| from .models import Job | ||
|
|
||
|
|
||
| def list_jobs(request): |
There was a problem hiding this comment.
This would be changed and just contain template response, since the views just calls the relevant API to fetch data.
|
@CuriousLearner heads up -> we are moving to |
we should really have details about this (and other code standards which we follow) in the contributor guidelines, so that people have the opportunity to know upfront, before they submit the PR |
Codecov Report
@@ Coverage Diff @@
## master #1102 +/- ##
=========================================
Coverage ? 15.32%
=========================================
Files ? 141
Lines ? 10973
Branches ? 1457
=========================================
Hits ? 1682
Misses ? 9285
Partials ? 6
Continue to review full report at Codecov.
|
|
@CuriousLearner Please follow the instructions in https://github.com/gitcoinco/web/blob/master/docs/CONTRIBUTING.md to enable |
|
thanks @CuriousLearner i will comment on the other thread.. when do you think youll have this PR ready for review? |
| /* eslint no-redeclare: "warn" */ | ||
|
|
||
| var build_detail_page = function(result) { | ||
| var template = $.templates('#job_detail'); |
There was a problem hiding this comment.
Expected indentation of 2 spaces but found 3. (indent)
| var build_detail_page = function(result) { | ||
| var template = $.templates('#job_detail'); | ||
|
|
||
| html = template.render(result); |
There was a problem hiding this comment.
Expected indentation of 2 spaces but found 3. (indent)
|
|
||
| html = template.render(result); | ||
|
|
||
| $('#rendered_job_detail').html(html); |
There was a problem hiding this comment.
Expected indentation of 2 spaces but found 3. (indent)
| var nonefound = true; | ||
|
|
||
| if (result) { | ||
| nonefound = false; |
There was a problem hiding this comment.
Expected indentation of 6 spaces but found 7. (indent)
|
|
||
| if (result) { | ||
| nonefound = false; | ||
| build_detail_page(result); |
There was a problem hiding this comment.
Expected indentation of 6 spaces but found 8. (indent)
| if (result) { | ||
| nonefound = false; | ||
| build_detail_page(result); | ||
| document.result = result; |
There was a problem hiding this comment.
Expected indentation of 6 spaces but found 8. (indent)
| nonefound = false; | ||
| build_detail_page(result); | ||
| document.result = result; | ||
| $('#rendered_job_detail').css('display', 'block'); |
There was a problem hiding this comment.
Expected indentation of 6 spaces but found 8. (indent)
|
|
||
| var main = function() { | ||
| setTimeout(function() { | ||
| pull_job_from_api(); |
There was a problem hiding this comment.
Expected indentation of 4 spaces but found 6. (indent)
app/assets/v2/js/pages/jobs.js
Outdated
| // } else if (result['fulfiller_address'] !== '0x0000000000000000000000000000000000000000') { | ||
| // result['my_bounty'] = '<a class="btn font-smaller-2 btn-sm btn-outline-dark" role="button" href="#">' + result['status'] + '</span></a>'; | ||
| // } | ||
| console.log(result); |
There was a problem hiding this comment.
Unexpected console statement. (no-console)
app/assets/v2/js/pages/jobs.js
Outdated
|
|
||
| result['job_company'] = ((result['company'] ? result['company'] : 'Company Hidden') + ' • '); | ||
|
|
||
| result['job_skill'] += result['skills'] ? result['skills'] : '' |
|
@CuriousLearner I could be mistaken, as I didn't scope this initially, but I think we're using |
|
yes |
|
Update: Right now working on initial form for job creation.
|
app/jobs/models.py
Outdated
| from datetime import timedelta | ||
|
|
||
| from django.conf import settings | ||
| from django.contrib.postgres.fields import ArrayField |
There was a problem hiding this comment.
F401 'django.contrib.postgres.fields.ArrayField' imported but unused
|
Sure, I've used skills API helper to populate the choice field for skills. I do have a question though:
|
|
Also, I see in the Job posting design that Job Type can be Full-time or Part-time, but the list view also shows a third job_type that says contract. Do we need to update one of the designs here? |
|
@thelostone-mc Can you take a look now please? :) |
|
testing now |
app/assets/v2/js/pages/jobs.js
Outdated
| var toggleAny = function(event) { | ||
| if (!event) | ||
| return; | ||
| // var key = event.target.name; |
There was a problem hiding this comment.
whats with this commented out code?
app/assets/v2/js/pages/jobs.js
Outdated
| }); | ||
| } | ||
|
|
||
| if (localStorage['keywords']) { |
There was a problem hiding this comment.
this will be a name collision with the dashboard.js which also has localStorage['keywords
ghost
assigned
| @@ -0,0 +1,3 @@ | |||
| from django.test import TestCase | |||
There was a problem hiding this comment.
F401 'django.test.TestCase' imported but unused
|
here is my QA notes: CRITICAL:
HIGH PRIORITY
OTHER
@CuriousLearner let me know what you think of the above. we're hoping to get this shipped in the next week or so, so let us know what your availability is to work through the issues |
app/assets/v2/css/jobs.css
Outdated
| font-size: 3em; | ||
| } | ||
|
|
||
| .company-name, #exp-lang, #job-post-duration { |
There was a problem hiding this comment.
could we break this up into multiple lines?
| padding-left: 1em; | ||
| padding-right: 2em; | ||
| } | ||
| } |
There was a problem hiding this comment.
Could we replace px and em with -> rem
app/dashboard/views.py
Outdated
| @@ -935,7 +935,7 @@ def profile_keywords_helper(handle): | |||
| """ | |||
| profile = profile_helper(handle, True) | |||
|
|
|||
| keywords = [] | |||
| keywords = list() | |||
There was a problem hiding this comment.
You shouldn't need to change this and is a commonly considered an antipattern in Python in favor of the shorthand []
|
|
||
| def job_detail(request, pk): | ||
| context = { | ||
| 'pk': pk |
There was a problem hiding this comment.
Should we handle any kind of update functionality against details and use the form you've already created?
| @@ -0,0 +1,111 @@ | |||
| {% comment %} | |||
There was a problem hiding this comment.
This template should extend base.html
| @@ -0,0 +1,122 @@ | |||
| {% load i18n static %} | |||
| <!DOCTYPE html> | |||
There was a problem hiding this comment.
This template should extend base.html
| @@ -0,0 +1,120 @@ | |||
| {% comment %} | |||
There was a problem hiding this comment.
This template should extend base.html
| verbose_name=_('Title'), max_length=200, null=False, blank=False | ||
| ) | ||
| description = models.TextField(verbose_name=_('Description')) | ||
| github_profile_link = models.URLField( |
There was a problem hiding this comment.
Github profiles should link directly back to dashboard.Profile versus char/url storage of text representing a Profile.
app/jobs/models.py
Outdated
| company = models.CharField(_('Company'), max_length=50, null=True, blank=True) | ||
| apply_email = models.EmailField(_('Contact Email for Job'), null=True, blank=True) | ||
| posted_at = models.DateTimeField(_('Posted At'), null=False, blank=False, default=timezone.now) | ||
| posted_by_gitcoin_username = models.CharField( |
There was a problem hiding this comment.
This should be associated with the dashboard.Profile that created the Job versus charfield of username
| @@ -0,0 +1,24 @@ | |||
|
|
|||
| # Third Party Stuff | |||
app/jobs/views.py
Outdated
|
|
||
|
|
||
| def list_jobs(request): | ||
| context = dict() |
There was a problem hiding this comment.
Drop this and simply context={} if that's what you're going for here. Realistically, we probably want to pass some contextual data to the list view.
app/jobs/views.py
Outdated
|
|
||
|
|
||
| def create_job(request): | ||
| context = dict() |
There was a problem hiding this comment.
When possible, please use the shorthand {}
|
@CuriousLearner let us know what you think of the above review! |
* 'fix-958' of github.com:CuriousLearner/web: linter
| @@ -0,0 +1,3 @@ | |||
| from django.test import TestCase | |||
There was a problem hiding this comment.
F401 'django.test.TestCase' imported but unused
| @@ -0,0 +1,3 @@ | |||
| from django.test import TestCase | |||
There was a problem hiding this comment.
F401 'django.test.TestCase' imported but unused
* 'master' of https://github.com/gitcoinco/web: (76 commits) hack: trailing slash for profile (gitcoinco#1937) Minor tweaks to action plan textarea (gitcoinco#1856) fixes label on gas guzzler page, and adds an admin view for vizuailzations keep an eye on unsubscribes in preferences keep an eye on unsubscribes in preferences Split create_new_bounty kwargs building and check if once (gitcoinco#1892) press satisfy linter fix for https://gitcoincore.slack.com/archives/CAXQ7PT60/p1533912000000126 Update profile check to getattr share link not edit link does tip receive math in BigNumber, bc JS rounding issues copy updates newsletter 8/10 fix for slack not showing UP gas guzzlers linter gas guzzlers Error: insufficient funds for gas * price + value starts to track gas guzzlers, for later inclusion in the gitcoin gas station ...
| @@ -0,0 +1,3 @@ | |||
| from django.test import TestCase | |||
There was a problem hiding this comment.
F401 'django.test.TestCase' imported but unused
| @@ -0,0 +1,3 @@ | |||
| from django.test import TestCase | |||
There was a problem hiding this comment.
F401 'django.test.TestCase' imported but unused
| @@ -0,0 +1,3 @@ | |||
| from django.test import TestCase | |||
There was a problem hiding this comment.
F401 'django.test.TestCase' imported but unused
|
Is this something we should pursue rebasing and handing back off to @CuriousLearner (if interested) or bountying back out for completion? cc @owocki @PixelantDesign @thelostone-mc @SaptakS |
|
i'd be curious to hear from @CuriousLearner if is interested in handling it.. if not, i think we backoff for now |
|
@CuriousLearner Thanks for the heads up - Closing for now. Feel free to re-open if/when you find the time! |
Description
This is still a WIP. Will update it soon.
Checklist
Affected core subsystem(s)
Testing
Refers/Fixes
Refs #958