-
Notifications
You must be signed in to change notification settings - Fork 153
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
(DO NOT MERGE YET) Fellowships work #996
Conversation
* Set stage for fellowships pages w/ django template
* Adding some stub urls for the fellowships site. * Update some of the urls a bit more
* Test commit * Find out which url is it resolving to * Fix stuff * Remove debugging stuff
Fellowships support page
* updated fellowships homepage
@@ -30,6 +30,9 @@ | |||
|
|||
urlpatterns = list(filter(None, [ | |||
url(r'^admin/', include(admin.site.urls)), | |||
url(r'^soc/', include('social_django.urls', namespace='social')) | |||
if settings.SOCIAL_SIGNIN else '', | |||
url(r'^fellowships/', include('networkapi.fellows.urls')), |
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.
on line 47 below, there's this code:
url(r'^soc/', include('social_django.urls', namespace='social')),
# Don't remove this redirect until Fellowships pages are live
url(r'^fellowships/$', RedirectView.as_view(
url='https://advocacy.mozilla.org/open-web-fellows'
))
if settings.SOCIAL_SIGNIN else None,
There are two things we need to look at here:
Firstly, we'll want to update the route pattern to something like this
url(r'^fellowship/(?P<path>.*)', RedirectView.as_view(
url='/fellowships/%(path)s',
query_string=True
)),
Secondly the line with
if settings.SOCIAL_SIGNIN else None,
should actually follow
url(r'^soc/', include('social_django.urls', namespace='social'))
... but there are two of the above url configs in the file, so lets de-dupe that!
* Related to #1116 - content changes to apply page
* Fixed #1128 - fixed broken 'see work' link
* redirected /fellowship to /fellowships * removed an extra line
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 you remove fellows/admin.py
, fellows/tests.py
, fellows/models.py
I think we should also file a followup to write tests for the views.
source/js/fellowships.js
Outdated
|
||
let pulseApiDomain = ``; | ||
let pulseDomain = ``; | ||
const DIRECOTRY_PAGE_FILTER_OPTIONS = {'program_year': `2017`}; |
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.
DIRECOTRY_PAGE_FILTER_OPTIONS
=> DIRECTORY_PAGE_FILTER_OPTIONS
source/js/fellowships.js
Outdated
let queryString = Object.entries(params).map(pair => pair.map(encodeURIComponent).join(`=`)).join(`&`); | ||
let req = new XMLHttpRequest(); | ||
|
||
req.addEventListener(`load`, () => { |
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.
This could be a one-liner:
req.addEventListener(
load, () => callback.call(this, JSON.parse(req.response)));
Do we really need to bind the callback to this
?
* code improvement * removed a few empty files
I'm opening up a PR to see if review app for the
fellowships
branch works.