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

Vueify config-postprocessing #4259

Merged
merged 99 commits into from
Aug 1, 2018
Merged

Conversation

p0psicles
Copy link
Contributor

@p0psicles p0psicles commented May 27, 2018

  • 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

Depends on:

Todo:

  • fix metadata tab
  • Use the same saved message as in editShow
  • Move vue components to vue-loader
  • Double check responsiveness
  • Update openApi specs
  • update changelog

@@ -0,0 +1,378 @@
<script type="text/x-template" id="name-pattern-tempate">
<div id="name_pattern_wrapper">

Copy link
Collaborator

Choose a reason for hiding this comment

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

Was this extra line intentional?

</label>
</div>
<div id="naming_key" class="nocheck" v-if="showLegend">
<table class="Key">
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be using 2 spaces like the rest of the HTML.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ive used 4 spaces for all now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah, this is 8 I think, double indented?

}
},
mounted() {
this.namingPattern = this.pattern;
Copy link
Collaborator

@OmgImAlexis OmgImAlexis May 28, 2018

Choose a reason for hiding this comment

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

Please switch to this.

const { pattern } = this;
this.namingPattern = pattern;

Copy link
Collaborator

Choose a reason for hiding this comment

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

You could also add namingPattern and presets here so you're not setting anything directly to this.*.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But why? You go from one line to two lines of code. What is the benefit of this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just the same reason you'd use a const anywhere else.

If you use this. you can still set it to another value which is wrong since you should be using the store or local data. If you're using a const there's no issue there and it makes it more clear that you're import read-only data.

@@ -290,16 +309,16 @@ const startVue = () => {
<fieldset class="component-group-list">

<!-- default name-pattern component -->
<name-pattern :pattern="pattern" :presets="presets"></name-pattern>
<name-pattern :enabled="true" :naming-pattern="pattern" :naming-presets="presets" :multi-ep-style="multiEpSelected" :multi-ep-styles="multiEpStrings" @change="onChangePattern" ></name-pattern>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this is the default why not move this data to the components data object as default values?

@p0psicles p0psicles changed the title Moved over files from search templates pr. Vueify config-postprocessing May 30, 2018
@p0psicles p0psicles mentioned this pull request Jun 8, 2018
@OmgImAlexis OmgImAlexis mentioned this pull request Jun 8, 2018
…eify-postprocessing

# Conflicts:
#	medusa/server/api/v2/config.py
#	themes-default/slim/views/config_postProcessing.mako
#	themes-default/slim/views/layouts/main.mako
#	themes/dark/templates/config_postProcessing.mako
#	themes/dark/templates/layouts/main.mako
#	themes/light/templates/config_postProcessing.mako
#	themes/light/templates/layouts/main.mako
… component.

Moved mako python to apiv2 / vue.
Started bootstrapping the postprocessing page.
<span class="component-title">Metadata Type:</span>
<span class="component-desc">
<span>Metadata Type:</span>
<span >
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missed a space here.

@@ -92,6 +92,19 @@ def get(self, identifier, path_param=None):

for section in config_sections:
config_data[section] = DataGenerator.get_data(section)
config_data['postProcessing'] = NonEmptyDict()
Copy link
Contributor

Choose a reason for hiding this comment

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

@p0psicles Don't forget to move these down to the DataGenerator class below.

@OmgImAlexis OmgImAlexis added this to the backlog milestone Jun 24, 2018
@OmgImAlexis
Copy link
Collaborator

@p0psicles any chance this could make it into the next release?

@p0psicles
Copy link
Contributor Author

p0psicles commented Jun 24, 2018

I think 0.2.7 is realistic
Most of the work will be migrating meta data tab

@p0psicles
Copy link
Contributor Author

Hmm that should not have made any difference in snapshot

@sharkykh
Copy link
Contributor

I think I made some changes and didn't update snapshots, sorry.

@p0psicles
Copy link
Contributor Author

But your changes still build. After my commit it errored?

@sharkykh
Copy link
Contributor

@p0psicles
You're right.
The diff is this block not longer shows up:

        <div class="form-group row">
            <h3 class="col-sm-12">Multi-EP sample:</h3>
            <div class="example col-sm-12">
                <span id="naming_example_multi" class="jumbo"></span>
            </div>
        </div>

@p0psicles
Copy link
Contributor Author

Waaat.. I checked that after i made the change. Hmm. Maybe it was still cached

@p0psicles
Copy link
Contributor Author

Happy to see the snapshot test is doing its work

@p0psicles
Copy link
Contributor Author

I had to do allot more testing on the name-pattern. And fixed some small bugs. It's still not perfect. But working and not making too many api calls. @sharkykh can you do a final check?

OmgImAlexis
OmgImAlexis previously approved these changes Jul 30, 2018
…ages that have not been made responsive yet.
…al post processing settings.

* Only show/hide scheduled pp settings with the "scheduled postprocessor" toggle.
* used the same bs classes for all the tabs.
* Reduced the min-height for component-group class to 100px
* Made tab metadata responsive.
OmgImAlexis
OmgImAlexis previously approved these changes Jul 31, 2018
<p>Settings that dictate how Medusa should process completed downloads.</p>
<p>The scheduled postprocessor while periodically scan a folder for media to process.</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Replace "while" with "will".

@OmgImAlexis OmgImAlexis removed Needs review Needs test (Vue) Needs tests added for this change (Vue) Needs testing Requires testing to make sure it's working as intended labels Jul 31, 2018
* Hide the name-pattern legend, when changing from custom pattern to preset.
@p0psicles
Copy link
Contributor Author

Sorry, i have no idea why the backend test is failing?

@p0psicles
Copy link
Contributor Author

Ok great. With one more approval this can be merged.

@p0psicles p0psicles merged commit 7daab93 into develop Aug 1, 2018
@p0psicles p0psicles deleted the feature/vueify-postprocessing branch August 1, 2018 01:10
@sharkykh sharkykh mentioned this pull request Aug 5, 2018
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.

3 participants