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 watched null values. #5132

Merged
merged 11 commits into from
Sep 9, 2018
Merged

Fix watched null values. #5132

merged 11 commits into from
Sep 9, 2018

Conversation

p0psicles
Copy link
Contributor

@p0psicles p0psicles commented Sep 7, 2018 •
edited
Loading

  • 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

@sharkykh still not sure if this PR is needed at all. But throwing it out here, just in case.

@p0psicles p0psicles requested a review from sharkykh September 7, 2018 06:03
OmgImAlexis
OmgImAlexis reviewed Sep 7, 2018
View reviewed changes
medusa/databases/main_db.py Outdated
@@ -221,6 +222,10 @@ def fix_subtitles_codes(self):
def fix_show_nfo_lang(self):
self.connection.action("UPDATE tv_shows SET lang = '' WHERE lang = 0 OR lang = '0';")

def fix_tv_episodes_watched_field(self):
"""A mistake was made when we added this field. Migration didn't went very well."""
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might want to swap "went" with "go".

@sharkykh
Copy link
Contributor

sharkykh commented Sep 7, 2018 •
edited
Loading

Where have you seen null values?
Edit: Fixes #3451 (comment)

@p0psicles p0psicles closed this Sep 7, 2018
@sharkykh sharkykh reopened this Sep 7, 2018
@sharkykh sharkykh added the Bug label Sep 7, 2018
p0psicles
p0psicles commented Sep 7, 2018
View reviewed changes
Copy link
Contributor Author

@p0psicles p0psicles left a comment

Choose a reason for hiding this comment

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

Thinking out loud, should we add a new migration?

@sharkykh
Copy link
Contributor

sharkykh commented Sep 7, 2018 •
edited
Loading

@p0psicles
IMO we really have to put a hold on migrations until the migration issues are solved...
I am still not sure about this fix too. I mean it should fix it...

@p0psicles
Copy link
Contributor Author

For new users yes. Users that are allready on db 4.12 not.

@sharkykh
Copy link
Contributor

sharkykh commented Sep 7, 2018 •
edited
Loading

@p0psicles
Why? I think sanity checks run every time Medusa starts up?

Medusa/medusa/__main__.py

Lines 1113 to 1114 in 6779024

main_db_con = db.DBConnection()
db.sanityCheckDatabase(main_db_con, main_db.MainSanityCheck)

@sharkykh sharkykh added Discussion Changelog Requires a changelog entry labels Sep 7, 2018
@sharkykh sharkykh added this to the 0.2.10 (Hotfix) milestone Sep 8, 2018
@sharkykh sharkykh mentioned this pull request Sep 8, 2018
Merged
8 tasks
@sharkykh
Revert all previous commits
40289db 
Revert "Update current sanity check and column default"

This reverts commit 6779024.

Revert "Update main_db.py"

This reverts commit a1c8e7a.

Revert "Update main_db.py"

This reverts commit a08759a.

Revert "Fix watched null values."

This reverts commit 848c527.
@sharkykh
Merge branch 'develop' into feature/fix-watched-null
a18b24a
@sharkykh
Use try_int when loading from database
6faa3de
@sharkykh
Episode.watched should be boolean
b96fc66
@sharkykh
Update changelog
a30e1d1
@sharkykh
Copy link
Contributor

sharkykh commented Sep 8, 2018 •
edited
Loading

I don't know why I didn't think of this before.
Episode.watched should be boolean anyway, so we should be using bool on the value.

I think the NULL values should fix themselves when the show is saved to the database,
because you're setting it to the value of self.watched which will be a boolean
(is it automatically converted to integer?)

@sharkykh sharkykh added Concluded Needs review and removed Changelog Requires a changelog entry Discussion labels Sep 8, 2018
sharkykh
sharkykh previously approved these changes Sep 8, 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.

I'm basically approving my own code so it can be merged

Please check my last comment.

@p0psicles
Copy link
Contributor Author

I also cant merge it

@p0psicles
Copy link
Contributor Author

Yeah the bool does the trick

@p0psicles p0psicles merged commit 31171bc into develop Sep 9, 2018
@p0psicles p0psicles deleted the feature/fix-watched-null branch September 9, 2018 10:41
p0psicles added a commit that referenced this pull request Sep 9, 2018
@p0psicles
Fix watched null values. (#5132)
0796b5a 
* Fix watched null values.

* Update main_db.py

oops

* Update main_db.py

* Update current sanity check and column default

* Revert all previous commits

Revert "Update current sanity check and column default"

This reverts commit 6779024.

Revert "Update main_db.py"

This reverts commit a1c8e7a.

Revert "Update main_db.py"

This reverts commit a08759a.

Revert "Fix watched null values."

This reverts commit 848c527.

* Use `try_int` when loading from database

* `Episode.watched` should be boolean

* Update changelog

# Conflicts:
#	CHANGELOG.md
#	medusa/tv/episode.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@OmgImAlexis OmgImAlexis OmgImAlexis approved these changes

@sharkykh sharkykh sharkykh left review comments

Assignees
No one assigned
Labels
Bug Concluded Needs review
Projects
None yet
Milestone
0.2.10
Development

Successfully merging this pull request may close these issues.
3 participants
@p0psicles @sharkykh @OmgImAlexis