Skip to content
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

Convert editShow to Vue store and SFC #4486

Merged
merged 63 commits into from Jun 29, 2019
Merged

Conversation

OmgImAlexis
Copy link
Collaborator

@OmgImAlexis OmgImAlexis commented Jun 28, 2018

TODO

  • Fix anidb UI
  • double check some weird bug in massUpdate when checking multiple shows and running update Can't reproduce it anymore ??

@sharkykh

This comment has been minimized.

Copy link
Contributor

@sharkykh sharkykh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just skimmed through...

themes-default/slim/views/editShow.mako Outdated Show resolved Hide resolved
themes-default/slim/views/editShow.mako Outdated Show resolved Hide resolved
themes-default/slim/views/editShow.mako Outdated Show resolved Hide resolved
@OmgImAlexis OmgImAlexis added this to the backlog milestone Aug 10, 2018
@p0psicles p0psicles modified the milestones: backlog, 0.2.9 Aug 21, 2018
@sharkykh sharkykh added the Changelog Requires a changelog entry label Aug 23, 2018
@sharkykh sharkykh modified the milestones: 0.2.9, 0.2.10 Aug 28, 2018
@sharkykh

This comment has been minimized.

@OmgImAlexis OmgImAlexis modified the milestones: 0.2.11, 0.2.12 Oct 29, 2018
@p0psicles p0psicles mentioned this pull request Jan 9, 2019
38 tasks
…eify-editShow

# Conflicts:
#	themes-default/slim/src/store/modules/shows.js
#	themes-default/slim/views/editShow.mako
#	themes/dark/assets/js/vendors.js
#	themes/dark/templates/editShow.mako
#	themes/light/assets/js/vendors.js
#	themes/light/templates/editShow.mako
* Fixed bug in concattenating globalRequired words.
* Removed mako vars from server/web/home/handler.py
* Updated router.js for edit-show.vue component.
* linting
@p0psicles
Copy link
Contributor

I'm hijacking this PR.

}

const { indexer, id, $store } = this;
$store.dispatch('setShow', { indexer, id, data, save: true }).then(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sharkykh I'm not totally sure. Should we call an action in vuex, and have that 1. update store then 2. update the show using apiv2? Or update the show from here, like it's now?

Copy link
Contributor

@sharkykh sharkykh Jun 11, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@p0psicles
TL;DR: The action should call the patch and fetch the show again.

I didn't look at the rest of the code, but since some of the patch data can be ignored by the API handler (invalid values and such), and whatever you send does not necessarily become the new show config, I'd say we can't update the store using the same data that we're sending, and we must fetch the show after the patch is complete.
I don't see a way around this at the moment.

return this.$store.state.search.filters.ignored.map(x => x.toLowerCase());
},
globalRequired() {
return this.$store.state.search.filters.required.map(x => x.toLowerCase());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was previously this.$store.state.search.filters.ignored.map(x => x.toLowerCase());

editShow is vueified and does not have the logic anymore to handle posts.
@p0psicles
Copy link
Contributor

@sharkykh can you do a yarn dev for me? I still have my build env messed up

@sharkykh
Copy link
Contributor

sharkykh commented Jun 28, 2019

Do you think we should merge it now?
Hopefully we didn't miss anything major...

Edit:
There's one thing I think we should be doing still -

$store.commit('currentShow', {
indexer,
id
});

The "show sub menu" makes use of the currentShow state.
I'll see if changing the code to use this is easy. Done.

@p0psicles
Copy link
Contributor

yeah lets merge

p0psicles
p0psicles previously approved these changes Jun 29, 2019
sharkykh
sharkykh previously approved these changes Jun 29, 2019
@sharkykh sharkykh dismissed their stale review June 29, 2019 16:42

found small bug

sharkykh
sharkykh previously approved these changes Jun 29, 2019
@p0psicles p0psicles merged commit 16558b0 into develop Jun 29, 2019
@p0psicles p0psicles deleted the feature/vueify-editShow branch June 29, 2019 17:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants