-
Notifications
You must be signed in to change notification settings - Fork 1
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
Ja vr private descriptions #176
base: master
Are you sure you want to change the base?
Conversation
…private descriptions for events that have them
Pull Request Test Coverage Report for Build 1145445894
💛 - Coveralls |
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.
Nice job on the form. I fixed the styling on the single event page but overall looks good! Also I would like for you guys to send this in the development channel on Slack so other people see you have an open PR.
…ve-bus-frontend into JA-VR-private-descriptions
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 💯 Small nitpicks here and there but looks solid overall.
dispatch(upcomingEvents.loading()); | ||
return publicApiClient | ||
.getUpcomingEvents() | ||
return eventsFetcher() |
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.
⛏️ Using a noun for a function name is generally bad, we would rather use a verb, i.e. fetchEvents()
return (dispatch, getState, { publicApiClient }) => { | ||
return (dispatch, getState, { publicApiClient, protectedApiClient }) => { | ||
const eventsFetcher: () => Promise<EventInformation[]> = | ||
getPrivilegeLevel(getState().authenticationState.tokens) === |
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.
⛏️ Not sure why we cant return this value directly and avoid defining a variable here.
Issue
Closes #163
Changes
Create/edit event forms should now include private descriptions
Frontend must display this field when registered users view the event on the single event view page