add join notifier support #5241

ouldsmobile · 2018-09-20T12:56:57Z

PR is based on the DEVELOP branch
Don't send big changes all at once. Split up big PRs into multiple smaller PRs that are easier to manage and review
Read the contribution guide

ouldsmobile · 2018-09-20T17:41:18Z

Copy/paste

fixed

ouldsmobile · 2018-09-20T17:41:31Z

Why did you change this to double quotes? We use single for strings.

fixed

p0psicles · 2018-09-20T18:25:59Z

Please fix all the issues pointed out by the travis build.
Frontend: xo/eslint js linting. Don't forget to run yarn dev
backend: pep
dredd: probably have to check apiv2 and dredd/api-description.yml

Ow and the PR is still subjected to a proper review. I just pointed out the obvious issues.

ouldsmobile · 2018-09-21T15:20:01Z

Please fix all the issues pointed out by the travis build.
Frontend: xo/eslint js linting. Don't forget to run yarn dev
backend: pep
dredd: probably have to check apiv2 and dredd/api-description.yml

Ow and the PR is still subjected to a proper review. I just pointed out the obvious issues.

Thanks for the help. Think I got it sorted now. As you can probably guess I am quite new to github and contributing code. Just wanted to contribute something to the project. Hope it helps!

OmgImAlexis · 2018-09-21T22:24:18Z

medusa/notifiers/join.py

@@ -1,4 +1,8 @@
 # coding=utf-8
+# Author: Kevin Ould email: ouldsmobile1@gmail.com
+# Thanks to the contributors of the other notifier providers
+# for inspiration and a base to start from


Extra notes are unneeded.

p0psicles · 2018-09-28T18:13:05Z

Ive decided that this will have to wait on #4913

Which means youll have to rewrite part of your code in vue.js.

ouldsmobile · 2018-09-29T13:38:11Z

Ive decided that this will have to wait on #4913

Which means youll have to rewrite part of your code in vue.js.

Ok, thanks for the update. So, once 4913 is merged I will update vue.js to work with new build. Will I be automatically notified when 4913 is merged or should I just keep checking on things? Sorry, again new to contributing to github. Still learning.

p0psicles · 2018-09-29T18:41:24Z

Will take at least another week. So you can check in next week.

p0psicles · 2018-10-30T08:37:37Z

You can rebase on develop now and continu

ghost · 2018-10-31T20:23:06Z

DeepCode analyzed this pull request.
There are 2 new info reports.

Click to see more details.

p0psicles · 2018-10-31T21:08:14Z

themes-default/slim/views/config_notifications.mako

@@ -1518,14 +1518,14 @@ window.app = new Vue({
                        <div class="col-xs-12 col-md-10">
                            <fieldset class="component-group-list">
                                <!-- All form components here for join client -->
-                                <config-toggle-slider :checked="notifiers.join.enabled" label="Enable" id="use_join" :explanations="['Send join notifications?']" @change="save()"  @update="notifiers.join.enabled = $event"></config-toggle-slider>
+                                <config-toggle-slider v-model="notifiers.join.enabled" label="Enable" id="use_join" :explanations="['Send join notifications?']" @change="save()"  @update="notifiers.join.enabled = $event"></config-toggle-slider>


You can also remove the @update now

medusa/server/web/config/notifications.py

themes/dark/assets/js/config/notifications.js

p0psicles · 2018-11-01T17:23:42Z

You need to run yarn dev, to fix the build

medusa/notifiers/join.py

themes-default/slim/src/store/modules/notifiers.js

themes-default/slim/views/config_notifications.mako

… unneeded line in config_notifications.mako

ghost · 2018-11-01T21:35:35Z

DeepCode analyzed this pull request.
There are no new issues.

ouldsmobile · 2018-11-02T14:15:30Z

Disregard, think I have it sorted now. Will update shortly.

…ldsmobile/Medusa into feature/join-notifier-support

ouldsmobile · 2018-11-02T19:28:45Z

Any idea why the frontend travis test keeps failing? All I can gather is the build-themes-check failed but not sure why? Seems to run fine locally with no errors.

p0psicles · 2018-11-02T20:15:51Z

Try removing node_modules
Run: yarn install --prod false --check-files

ouldsmobile · 2018-11-02T20:32:16Z

Try removing node_modules
Run: yarn install --prod false --check-files

Ok, ran that locally in root of build. Here is the tail:

`Callback’ {aka ‘void (*)(const v8::WeakCallbackInfo&)’} [-Wcast-function-type]
make: *** [protagonist.target.mk:155: Release/obj.target/protagonist/src/parse_async.o] Error 1

make: Leaving directory '/home/kevin/Medusa/node_modules/protagonist/build'

gyp ERR! build error

gyp ERR! stack Error: make failed with exit code: 2

gyp ERR! stack at ChildProcess.onExit (/home/kevin/.config/yarn/global/node_modules/node-
gyp/lib/build.js:262:23)

gyp ERR! stack at ChildProcess.emit (events.js:182:13)

gyp ERR! stack at Process.ChildProcess._handle.onexit (internal/child_process.js:240:12)

gyp ERR! System Linux 4.18.16-arch1-1-ARCH

gyp ERR! command "/usr/bin/node" "/home/kevin/.yarn/bin/node-gyp" "rebuild"

gyp ERR! cwd /home/kevin/Medusa/node_modules/protagonist

gyp ERR! node -v v11.0.0

gyp ERR! node-gyp -v v3.8.0

gyp ERR! not ok"

info This module is OPTIONAL, you can safely ignore this error

$ yarn run lint

yarn run v1.12.1

$ node ./themes-default/helper.js lint

Starting lint of /home/kevin/Medusa/themes-default/slim

$ /home/kevin/Medusa/themes-default/slim/node_modules/.bin/xo

Done in 1.71s.

Done in 90.59s.
`

p0psicles · 2018-11-02T21:12:13Z

You need to run that from /home/kevin/Medusa/themes-default/slim

ouldsmobile · 2018-11-02T22:20:02Z

You need to run that from /home/kevin/Medusa/themes-default/slim

Ah ok. Seemed to run without error. Here is the output from that:

`yarn install v1.12.1

warning package.json: No license field

warning slim@0.1.0: No license field

[1/4] Resolving packages...

[2/4] Fetching packages...

info fsevents@1.2.4: The platform "linux" is incompatible with this module.

info "fsevents@1.2.4" is an optional dependency and failed compatibility check. Excluding it from
installation.
[3/4] Linking dependencies...

[4/4] Building fresh packages...

$ yarn run lint

yarn run v1.12.1

warning package.json: No license field

$ xo

Done in 1.46s.

Done in 9.03s.
`

p0psicles · 2018-11-03T03:38:31Z

And now run from the same dir: yarn dev

ouldsmobile · 2018-11-03T11:37:45Z

And now run from the same dir: yarn dev

Thanks for the help! Passing checks now.

ouldsmobile · 2018-11-05T17:22:22Z

I noticed the schemes for notify_snatch, notify_download and notify_subtitle_download have changed since I originally wrote my join.py. notify_snatch has been taken care of with pr #5584 but the other two need to be fixed as well. I have edited and fixed in my repo. What is the proper way to merge the changes into develop now? Make another pull request? Or can you just make the necessary changes? I want to do things correctly.

ouldsmobile added 3 commits September 20, 2018 08:14

add join notifier support

c1450f0

Update join.py

f8934f6

Update __init__.py

abf8433

ouldsmobile added 10 commits September 20, 2018 15:05

fix bad indent in join.py

75000bf

fix bad formatting

377d863

fix bad formatting

3921354

fix bad formatting

b0525b9

fix more formatting errors

126f5a4

fix travis errors in join.py

9f26357

add missing docstrings in join.py

0c1ea0a

fixing docstrings in join.py

1f1790c

fixing docstrings in join.py

0e454ea

fixing docstrings in join.py

95fa1d2

medariox added Needs review Needs testing Requires testing to make sure it's working as intended labels Sep 21, 2018

OmgImAlexis reviewed Sep 21, 2018

View reviewed changes

OmgImAlexis requested a review from medariox September 21, 2018 22:24

removed unneccessary comments from join.py

4d0a3bd

OmgImAlexis added this to the 0.2.12 milestone Sep 29, 2018

p0psicles mentioned this pull request Oct 16, 2018

Add Join to Device Notfications #5428

Closed

ouldsmobile added 2 commits October 30, 2018 17:17

add join notifier support

9321d44

Update join.py

9d27eb8

update to v-model sliders, fix some minor edit errors

513f9c0

forgot to hit save during one of my edits. push was missing some updates

9b6c6b0

p0psicles reviewed Oct 31, 2018

View reviewed changes

removed @updates

b883c4f

p0psicles requested changes Nov 1, 2018

View reviewed changes

medusa/server/web/config/notifications.py Show resolved Hide resolved

themes/dark/assets/js/config/notifications.js Outdated Show resolved Hide resolved

remove notifications.js files from themes, no longer used

7dfb935

p0psicles requested changes Nov 1, 2018

View reviewed changes

medusa/notifiers/join.py Outdated Show resolved Hide resolved

themes-default/slim/src/store/modules/notifiers.js Outdated Show resolved Hide resolved

themes-default/slim/views/config_notifications.mako Outdated Show resolved Hide resolved

ouldsmobile added 4 commits November 1, 2018 14:35

remove notifiers.js files from themes-default, no longer used, remove…

252d42a

… unneeded line in config_notifications.mako

ran yarn

89482d7

update app.py from develop branch

70d3f99

Merge branch 'develop' into feature/join-notifier-support

07ff739

ouldsmobile added 4 commits November 2, 2018 14:31

ran yarn --dev

24af1e8

Merge branch 'feature/join-notifier-support' of https://github.com/ou…

82e7eea

…ldsmobile/Medusa into feature/join-notifier-support

removed unneeded url encoding fron join.py

7ece187

removed unneeded urllib import from join.py

50640be

reran yarn

7fadc35

p0psicles approved these changes Nov 3, 2018

View reviewed changes

p0psicles merged commit 676b11e into pymedusa:develop Nov 3, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add join notifier support #5241

add join notifier support #5241

ouldsmobile commented Sep 20, 2018 •

edited

Loading

ouldsmobile commented Sep 20, 2018

ouldsmobile commented Sep 20, 2018

p0psicles commented Sep 20, 2018 •

edited

Loading

ouldsmobile commented Sep 21, 2018

OmgImAlexis Sep 21, 2018

ouldsmobile Sep 21, 2018

p0psicles commented Sep 28, 2018

ouldsmobile commented Sep 29, 2018

p0psicles commented Sep 29, 2018

p0psicles commented Oct 30, 2018

ghost commented Oct 31, 2018

p0psicles Oct 31, 2018

ouldsmobile Oct 31, 2018

p0psicles commented Nov 1, 2018

ghost commented Nov 1, 2018

ouldsmobile commented Nov 2, 2018 •

edited

Loading

ouldsmobile commented Nov 2, 2018

p0psicles commented Nov 2, 2018

ouldsmobile commented Nov 2, 2018 •

edited

Loading

p0psicles commented Nov 2, 2018

ouldsmobile commented Nov 2, 2018

p0psicles commented Nov 3, 2018

ouldsmobile commented Nov 3, 2018

ouldsmobile commented Nov 5, 2018

add join notifier support #5241

add join notifier support #5241

Conversation

ouldsmobile commented Sep 20, 2018 • edited Loading

ouldsmobile commented Sep 20, 2018

ouldsmobile commented Sep 20, 2018

p0psicles commented Sep 20, 2018 • edited Loading

ouldsmobile commented Sep 21, 2018

OmgImAlexis Sep 21, 2018

Choose a reason for hiding this comment

ouldsmobile Sep 21, 2018

Choose a reason for hiding this comment

p0psicles commented Sep 28, 2018

ouldsmobile commented Sep 29, 2018

p0psicles commented Sep 29, 2018

p0psicles commented Oct 30, 2018

ghost commented Oct 31, 2018

p0psicles Oct 31, 2018

Choose a reason for hiding this comment

ouldsmobile Oct 31, 2018

Choose a reason for hiding this comment

p0psicles commented Nov 1, 2018

ghost commented Nov 1, 2018

ouldsmobile commented Nov 2, 2018 • edited Loading

ouldsmobile commented Nov 2, 2018

p0psicles commented Nov 2, 2018

ouldsmobile commented Nov 2, 2018 • edited Loading

p0psicles commented Nov 2, 2018

ouldsmobile commented Nov 2, 2018

p0psicles commented Nov 3, 2018

ouldsmobile commented Nov 3, 2018

ouldsmobile commented Nov 5, 2018

ouldsmobile commented Sep 20, 2018 •

edited

Loading

p0psicles commented Sep 20, 2018 •

edited

Loading

ouldsmobile commented Nov 2, 2018 •

edited

Loading

ouldsmobile commented Nov 2, 2018 •

edited

Loading