Add Events to Beta Scheduler & Improve Vue Code

vkoves commented 5 years ago

Description

This PR does a few notable things:

Adds a user#events action for pulling a user's events as JSON
Makes the beta scheduler show a list of the user's events
Adds linting to Vue via eslint-plugin-vue
Refactors the Vue code to follow more best practices - esp. around having components not do data retrieval.

You can view the beta scheduler on Carpe test.

This work pushes us further towards issue #217.

Type of Pull Request

Based on the contributor's guide, this PR is of type:

[x] Development (feature-branch -> dev)
[ ] Hotfix (hotfix-branch -> master)
[ ] Release (release-branch -> master)
[ ] Other

Requestor Checklist

Requestor: Put an x in all that apply. You can check boxes after the PR has been made.

Reviewer: If you see an item that is not checked that you believe should be, comment on that as part of your review.

[ ] Code Quality: I have written tests to ensure that my changes work and handle edge cases
[x] Code Quality: I have documented my changes thoroughly (using JSDoc in Javascript)
[x] Process: I have linked relevant issues, marking issues that this PR resolves
[x] Process: I have requested at least as many reviews required for this PR type (2 or 3)
[x] Process: I have added this PR to the relevant quarterly milestone
[x] Process: I have tested this PR locally and verified it does what it should

How This Has Been Tested

Tested manually and via ESLint.

vkoves commented 5 years ago

Note: This PR looks big, but 400 lines are just lock file changes, so this isn't actually unreasonable.

vkoves commented 5 years ago

CI was failing due to CircleCI using Node 8 instead of 10, but I haven't been able to fix it :sob:

Watercycle commented 5 years ago

@vkoves Now it's failing for legitimate reasons!

vkoves commented 5 years ago

@Watercycle - thank you so much! I don't know why it can't find ESLint Vue though, yarn install should cover that 😧

Watercycle commented 5 years ago

@vkoves I had a hunch it was either a lockfile issue or local vs global and gave that a try. Good to go! 🐋

Unrelated question. Has the "changing event gives guest a notification" test randomly failed for any of your past CI builds? I've seen it happen twice, but I don't see how that's possible - it's synchronous code.

vkoves commented 5 years ago

@Watercycle - you are the hero that we do not deserve :sob:

Also no, I haven't had that test failing randomly, but feel free to open an issue to investigate it. Also, should we not have the || true in our eslint run? I think we did that because otherwise NPM throws a bunch of errors in command line, which is super messy

Watercycle commented 5 years ago

@vkoves When I introduce an issue on my Ubuntu 18 WSL (Bash for Windows):

npm run eslint

root@WaterDesktop:/mnt/c/LinuxShared/carpe# npm run eslint

> carpe@1.0.0 eslint /mnt/c/LinuxShared/carpe
> eslint app/assets/javascripts app/javascript spec/javascripts --ext .js,.vue

/mnt/c/LinuxShared/carpe/app/javascript/scheduler.vue
  50:59  error  Missing semicolon  semi

✖ 1 problem (1 error, 0 warnings)
  1 error and 0 warnings potentially fixable with the `--fix` option.

npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! carpe@1.0.0 eslint: `eslint app/assets/javascripts app/javascript spec/javascripts --ext .js,.vue`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the carpe@1.0.0 eslint script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /root/.npm/_logs/2019-03-08T18_17_56_723Z-debug.log

npm run eslint --silent

root@WaterDesktop:/mnt/c/LinuxShared/carpe# npm run eslint --silent

/mnt/c/LinuxShared/carpe/app/javascript/scheduler.vue
  50:59  error  Missing semicolon  semi

✖ 1 problem (1 error, 0 warnings)
  1 error and 0 warnings potentially fixable with the `--fix` option.

Is that what you're referring to?

vkoves commented 5 years ago

@Watercycle - yes indeed! I get this huge output now with NPM:

Screenshot from 2019-03-10 13-40-14

However, I assume we need this so CI knows a failure happens, so oh well

Watercycle commented 5 years ago

@vkoves You still get those errors with the --silent flag?

vkoves commented 5 years ago

I don't, thank you! Since we need npm run eslint to return a failure if there's a linting failure, you are very right to suggest that.

vkoves commented 5 years ago

CI is passing, so it's merge time! :rocket:

vkoves / carpe