Add support of nfo when "Import existing series on Disk" #2096

j0nathan33 · 2017-08-03T17:09:20Z

Database Migration

NO

Description

Add some code to read nfo when do "Importing existing series on Disk"

Todos

Tests
Documentation

Issues Fixed or Closed by this PR

Fix issues #289

markus101

Thanks for picking this up, the current solution needs some work before we can accept it though. I'm not sure how familiar you are with the codebase or C# in general, but please let us know if you have any questions and we'll try to assist.

markus101 · 2017-08-03T17:20:45Z

src/NzbDrone.Core/RootFolders/RootFolderService.cs

@@ -153,7 +154,34 @@ private List<UnmappedFolder> GetUnmappedFolders(string path)
            foreach (string unmappedFolder in unmappedFolders)
            {
                var di = new DirectoryInfo(unmappedFolder.Normalize());
-                results.Add(new UnmappedFolder { Name = di.Name, Path = di.FullName });
+                var fileNfoName = String.Format("{0}\\tvshow.nfo", di.FullName);


Use Path.Combine to join paths, which works on different operating systems, currently this would break on anything other than Windows.

markus101 · 2017-08-03T17:21:14Z

src/NzbDrone.Core/RootFolders/RootFolderService.cs

@@ -153,7 +154,34 @@ private List<UnmappedFolder> GetUnmappedFolders(string path)
            foreach (string unmappedFolder in unmappedFolders)
            {
                var di = new DirectoryInfo(unmappedFolder.Normalize());
-                results.Add(new UnmappedFolder { Name = di.Name, Path = di.FullName });
+                var fileNfoName = String.Format("{0}\\tvshow.nfo", di.FullName);
+                if (File.Exists(fileNfoName))


Use DiskProvider.FileExists so it can be mocked for unit tests.

markus101 · 2017-08-03T17:22:28Z

src/NzbDrone.Core/RootFolders/RootFolderService.cs

+                        else
+                        {
+                            String tvid = node.InnerText;
+                            results.Add(new UnmappedFolder { Name = String.Format("tvdb:{0}", tvid), Path = di.FullName });


The name should still be the name, any additional information should be returned via a separate property, which also means the API and UI will need to be updated to pull in that information.

markus101 · 2017-08-03T17:25:49Z

src/NzbDrone.Core/RootFolders/RootFolderService.cs

+                var fileNfoName = String.Format("{0}\\tvshow.nfo", di.FullName);
+                if (File.Exists(fileNfoName))
+                {
+                    try


The logic of pulling the ID from an NFO file should be pulled out to a separate method, possibly another class/interface so additional metadata types could be also be handled. It'd be worth looking at extending the existing metadata handling to be able to check for an existing file and check that file for an ID if the file doesn't exist or doesn't support storing an an ID Sonarr can use for a lookup it would return null.

…rt-read-nfo-from-import

j0nathan33 · 2017-08-08T16:43:56Z

I don't know why junit fail when I only edit javascript file

markus101

A few more things to address. Extracting the ID from the file in RootFolderServer still seems like the wrong approach, since it puts specific knowledge about the file there, when it otherwise shouldn't care. We'll need tests to validate the behaviour if it's going to stay here in RootFolderServiceFixture.

As for the unit tests failing when you made a change to the JS it's because the whole test suite was re-run (it does that every build regardless of what changed).

markus101 · 2017-08-09T19:01:42Z

src/UI/AddSeries/Existing/AddExistingSeriesCollectionView.js

@@ -30,9 +30,12 @@ module.exports = Marionette.CompositeView.extend({

        if (model) {
            var currentIndex = index;
-            var folderName = model.get('folder').name;
+            var termSearch = model.get('folder').name;
+            if (typeof model.get('folder').id !== "undefined") {


model.get('folder').id != null (instead of the !== you had before as != will catch undefined and null.

markus101 · 2017-08-09T19:04:13Z

src/NzbDrone.Core/RootFolders/UnmappedFolder.cs

@@ -4,5 +4,6 @@ public class UnmappedFolder
    {
        public string Name { get; set; }
        public string Path { get; set; }
+        public string Id { get; set; }


Id is reserved for the Id of the record, TvdbId would be more accurate since that assumption is being made in the client. It should also be an integer instead of a string. Probably a nullable int so we don't return 0 if it's unknown.

markus101 · 2017-08-09T19:05:23Z

src/NzbDrone.Core/RootFolders/RootFolderService.cs

+                    XmlNode node = doc.DocumentElement.SelectSingleNode("/tvshow/id");
+                    if (node != null)
+                    {
+                        returnValue = node.InnerText;


Why not return directly from here? This should also parse it to an integer to ensure it's valid.

markus101 · 2017-08-09T21:20:38Z

After talking with @Taloth there are a couple things we want to improve here:

The logic for pulling the TVDB ID needs to be moved outside of the RootFolderService
The host should tell the client what to search for, that is, if it has a TVDB ID, it should set the search term to tvdbid:12345 and if it doesn't it should return the folder name, the client will then use the search term from the server and not decide which to use on it's own

Add support of nfo when "Import existing series on Disk"

d786d20

markus101 requested changes Aug 3, 2017

View reviewed changes

j0nathan33 added 5 commits August 3, 2017 17:39

Refactorise after codeReview

2ade1bb

Forgot to add ";"

a86ced1

Merge branch 'develop' of https://github.com/Sonarr/Sonarr into suppo…

c060c31

…rt-read-nfo-from-import

Fix warning from npm

dff6ef7

Fix variable when is not available from api

365a761

markus101 requested changes Aug 9, 2017

View reviewed changes

markus101 closed this Nov 30, 2018

github-actions bot locked as resolved and limited conversation to collaborators Jan 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support of nfo when "Import existing series on Disk" #2096

Add support of nfo when "Import existing series on Disk" #2096

j0nathan33 commented Aug 3, 2017 •

edited

markus101 left a comment

markus101 Aug 3, 2017

markus101 Aug 3, 2017

markus101 Aug 3, 2017

markus101 Aug 3, 2017

j0nathan33 commented Aug 8, 2017

markus101 left a comment

markus101 Aug 9, 2017

markus101 Aug 9, 2017

markus101 Aug 9, 2017

markus101 commented Aug 9, 2017

Add support of nfo when "Import existing series on Disk" #2096

Add support of nfo when "Import existing series on Disk" #2096

Conversation

j0nathan33 commented Aug 3, 2017 • edited

Database Migration

Description

Todos

Issues Fixed or Closed by this PR

markus101 left a comment

Choose a reason for hiding this comment

markus101 Aug 3, 2017

Choose a reason for hiding this comment

markus101 Aug 3, 2017

Choose a reason for hiding this comment

markus101 Aug 3, 2017

Choose a reason for hiding this comment

markus101 Aug 3, 2017

Choose a reason for hiding this comment

j0nathan33 commented Aug 8, 2017

markus101 left a comment

Choose a reason for hiding this comment

markus101 Aug 9, 2017

Choose a reason for hiding this comment

markus101 Aug 9, 2017

Choose a reason for hiding this comment

markus101 Aug 9, 2017

Choose a reason for hiding this comment

markus101 commented Aug 9, 2017

j0nathan33 commented Aug 3, 2017 •

edited