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.

Sign up for GitHub

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

Jump to bottom

fix missing vue pages #3889

Merged
merged 11 commits into from
Mar 22, 2018
Merged

fix missing vue pages #3889

merged 11 commits into from
Mar 22, 2018

Conversation

OmgImAlexis
Copy link
Collaborator

Closes #3888

@OmgImAlexis OmgImAlexis requested a review from medariox March 21, 2018 22:13
@codecov-io
Copy link

codecov-io commented Mar 21, 2018 •
edited
Loading

Codecov Report

Merging #3889 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #3889   +/-   ##
========================================
  Coverage    29.37%   29.37%           
========================================
  Files          274      274           
  Lines        35420    35420           
========================================
  Hits         10403    10403           
  Misses       25017    25017

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1d62c90...5eefe29. Read the comment docs.

@OmgImAlexis OmgImAlexis force-pushed the fix/missing-vue-pages branch from 4f92dc8 to 384e8e2 Compare March 21, 2018 22:44
sharkykh
sharkykh approved these changes Mar 21, 2018
View reviewed changes
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.

Works for me

@sharkykh
Copy link
Contributor

sharkykh commented Mar 21, 2018 •
edited
Loading

Can you check the /errorlogs/ and /addRecommended/ routes? They seem broken to me as well:
image

@sharkykh
Copy link
Contributor

sharkykh commented Mar 21, 2018 •
edited
Loading

Also found this is broken (at least those two lines):
https://github.com/pymedusa/Medusa/pull/3884/files#diff-c05bb2c59f6b274b8c5134313046b898R51

Error is:

SyntaxException: (SyntaxError) EOL while scanning string literal (<unknown>, line 1) (u'\'realpage + \'/\'}?traktList=${traktList}#tabs-1">Ma') in file 'C:/Medusa/themes/light/templates/addShows_trendingShows.mako' at line: 51 char: 75

Same for addShows_recommended.mako

heh I keep thinking it's the last one then I find one more, sorry!

@sharkykh
Copy link
Contributor

sharkykh commented Mar 21, 2018 •
edited
Loading

I have a show with a double quotation mark in its name - תאג"ד, since the last PR it broke some stuff:

Shows list:
before
after

Show page:
before
after

@sharkykh
Copy link
Contributor

sharkykh commented Mar 21, 2018 •
edited
Loading

Not sure if directly related to my last comment - recent shows links are broken for me:
I get the link - https://localhost:8081/home/displayShow?indexername=indexer-to-name&seriesid=316913.
indexer-to-name should be tvdb for me.
Page source shows:

<li>
    <a href="https://khpc:8081/home/displayShow?indexername=indexer-to-name&amp;seriesid=316913" target="_self" data-indexer-to-name="1">
        <i class="menu-icon-addshow"></i>
        &nbsp;תאג"ד
    </a>
</li>

@OmgImAlexis
Copy link
Collaborator Author

In regards to #3889 (comment) it looks like @p0psicles didn't add the function in?

Ref: https://github.com/pymedusa/Medusa/blame/master/themes/dark/templates/partials/header.mako#L36

@OmgImAlexis
Copy link
Collaborator Author

@sharkykh that should be everything fixed. Can you please review.

@sharkykh
Copy link
Contributor

Still seeing an error in addShows_recommended.mako (lines 51-52) and the character escape issue in partials/showheader.mako.
Apart from that and the recent shows, LGTM.

@sharkykh
Copy link
Contributor

By the way, it seems like the indexer-to-name stuff is injected using JS.
But it's late here, so I might be wrong.

@OmgImAlexis
Copy link
Collaborator Author

@sharkykh recent should be fine now.

@p0psicles I've removed your data-indexer-to-name function and moved the relevant code to the app-link so you can now just pass a indexer-id to app-link and it'll replace the placeholder with the correct value.

I also added placeholder as a prop so you can change the placeholder as it defaults to indexer-to-name.

@p0psicles p0psicles merged commit 527c232 into develop Mar 22, 2018
@p0psicles p0psicles deleted the fix/missing-vue-pages branch March 22, 2018 05:32
p0psicles pushed a commit that referenced this pull request Mar 22, 2018
@sharkykh @p0psicles
Template fixes (#3894)
a928239 
* Remove extra `${` from simple view

Caused error 500.

* Escape show names in simple, poster & small poster views

Fixes issues similar to #3889 (comment)

* fix 500.mako missing script block

* fix 404.mako missing script block
@OmgImAlexis OmgImAlexis mentioned this pull request Apr 20, 2018
Open
@p0psicles p0psicles added this to the 0.2.2 milestone Apr 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sharkykh sharkykh sharkykh approved these changes

@p0psicles p0psicles p0psicles approved these changes

@medariox medariox Awaiting requested review from medariox

Assignees
No one assigned
Labels
Bug Concluded
Projects
None yet
Milestone
0.2.2
Development

Successfully merging this pull request may close these issues.
4 participants
@OmgImAlexis @codecov-io @sharkykh @p0psicles