From 7cd2d610b13de3b964b6cbe44673b3b77a0c435a Mon Sep 17 00:00:00 2001
From: Keith Edmunds <kae@tiger-computing.co.uk>
Date: Sun, 13 Feb 2022 17:34:20 +0000
Subject: [PATCH] playlist.tracks now association object plus refactoring

---
 app/models.py  |  33 +++++---
 test_models.py | 209 +++++++++++++++++++++++++++++--------------------
 2 files changed, 146 insertions(+), 96 deletions(-)

diff --git a/app/models.py b/app/models.py
index 9d8462a..a897958 100644
--- a/app/models.py
+++ b/app/models.py
@@ -6,6 +6,7 @@ import re
 import sqlalchemy
 
 from datetime import datetime
+from sqlalchemy.ext.associationproxy import association_proxy
 from sqlalchemy.ext.declarative import declarative_base
 from sqlalchemy import (
     Boolean,
@@ -19,7 +20,7 @@ from sqlalchemy import (
 )
 from sqlalchemy.exc import IntegrityError
 from sqlalchemy.orm.exc import MultipleResultsFound, NoResultFound
-from sqlalchemy.orm import relationship, sessionmaker, scoped_session
+from sqlalchemy.orm import backref, relationship, sessionmaker, scoped_session
 
 from app.config import Config
 from app.log import DEBUG, ERROR
@@ -61,6 +62,18 @@ class NoteColours(Base):
     is_casesensitive = Column(Boolean, default=False, index=False)
     order = Column(Integer, index=True)
 
+    def __init__(self, session, substring, colour, enabled=True,
+                 is_regex=False, is_casesensitive=False, order=0):
+        self.substring = substring
+        self.colour = colour
+        self.enabled = enabled
+        self.is_regex = is_regex
+        self.is_casesensitive = is_casesensitive
+        self.order = order
+
+        session.add(self)
+        session.commit()
+
     def __repr__(self):
         return (
             f"<NoteColour(id={self.id}, substring={self.substring}, "
@@ -139,7 +152,7 @@ class Notes(Base):
 
         DEBUG(f"delete_note({self.id=}")
 
-        session.query(self).filter(id == self.id).delete()
+        session.query(Notes).filter_by(id=self.id).delete()
         session.commit()
 
     def update_note(self, session, row, text=None):
@@ -149,7 +162,7 @@ class Notes(Base):
 
         DEBUG(f"Notes.update_note({self.id=}, {row=}, {text=})")
 
-        note = session.query(self).filter(id == self.id).one()
+        note = session.query(Notes).filter_by(id=self.id).one()
         note.row = row
         if text:
             note.note = text
@@ -213,9 +226,8 @@ class Playlists(Base):
     notes = relationship("Notes",
                          order_by="Notes.row",
                          back_populates="playlist")
-    tracks = relationship("PlaylistTracks",
-                          order_by="PlaylistTracks.row",
-                          back_populates="playlists")
+
+    tracks = association_proxy('playlist_tracks', 'tracks')
 
     def __init__(self, session, name):
         self.name = name
@@ -240,7 +252,8 @@ class Playlists(Base):
             last_row = session.query(
                 func.max(PlaylistTracks.row)
             ).filter_by(playlist_id=self.id).first()
-            if last_row:
+            # if there are no rows, the above returns (None, ) which is True
+            if last_row and last_row[0]:
                 row = last_row[0] + 1
             else:
                 row = 0
@@ -319,14 +332,14 @@ class Playlists(Base):
 
 
 class PlaylistTracks(Base):
-    __tablename__ = 'playlisttracks'
+    __tablename__ = 'playlist_tracks'
 
     id = Column(Integer, primary_key=True, autoincrement=True)
     playlist_id = Column(Integer, ForeignKey('playlists.id'), primary_key=True)
     track_id = Column(Integer, ForeignKey('tracks.id'), primary_key=True)
     row = Column(Integer, nullable=False)
-    tracks = relationship("Tracks", back_populates="playlists")
-    playlists = relationship("Playlists", back_populates="tracks")
+    tracks = relationship("Tracks")
+    playlist = relationship(Playlists, backref=backref("playlist_tracks"))
 
     def __init__(self, session, playlist_id, track_id, row):
         DEBUG(f"PlaylistTracks.__init__({playlist_id=}, {track_id=}, {row=})")
diff --git a/test_models.py b/test_models.py
index 72c38e6..8a60e5d 100644
--- a/test_models.py
+++ b/test_models.py
@@ -5,7 +5,6 @@ from app.models import (
     Notes,
     Playdates,
     Playlists,
-    # PlaylistTracks,
     Tracks,
 )
 
@@ -13,34 +12,107 @@ from app.models import (
 def test_notecolours_get_colour(session):
     """Create a colour record and retrieve all colours"""
 
-    rec = NoteColours()
-    rec.colour = "ffffff"
-    rec.substring = "substring"
-    rec.is_regex = False
-    rec.is_casesensitive = True
-    session.add(rec)
-    session.commit()
+    note_colour = "#abcdef"
+    NoteColours(session, substring="substring", colour=note_colour)
 
     records = NoteColours.get_all(session)
     assert len(records) == 1
     record = records[0]
-    assert record.colour == "ffffff"
+    assert record.colour == note_colour
 
 
-def test_notecolours_get_by_id(session):
-    """Create a colour record and retrieve it by id"""
+def test_notecolours_get_all(session):
+    """Create two colour records and retrieve them all"""
 
-    rec = NoteColours()
-    rec.colour = "abcdef"
-    rec.substring = "substring"
-    rec.is_regex = False
-    rec.is_casesensitive = True
-    session.add(rec)
-    session.commit()
-    record_id = rec.id
+    note1_colour = "#abcdef"
+    note2_colour = "#00ff00"
+    NoteColours(session, substring="note1", colour=note1_colour)
+    NoteColours(session, substring="note2", colour=note2_colour)
 
-    record = NoteColours.get_by_id(session, record_id)
-    assert record.colour == "abcdef"
+    records = NoteColours.get_all(session)
+    assert len(records) == 2
+    assert note1_colour in [n.colour for n in records]
+    assert note2_colour in [n.colour for n in records]
+
+
+def test_notecolours_get_colour_none(session):
+    note_colour = "#abcdef"
+    NoteColours(session, substring="substring", colour=note_colour)
+
+    result = NoteColours.get_colour(session, "xyz")
+    assert result is None
+
+
+def test_notecolours_get_colour_match(session):
+
+    note_colour = "#abcdef"
+    nc = NoteColours(session, substring="sub", colour=note_colour)
+    assert nc
+
+    result = NoteColours.get_colour(session, "The substring")
+    assert result == note_colour
+
+
+def test_notes_creation(session):
+    # We need a playlist
+    playlist = Playlists(session, "my playlist")
+
+    note_text = "note text"
+    note = Notes(session, playlist.id, 0, note_text)
+    assert note
+
+    notes = session.query(Notes).all()
+    assert len(notes) == 1
+    assert notes[0].note == note_text
+
+
+def test_notes_delete(session):
+    # We need a playlist
+    playlist = Playlists(session, "my playlist")
+
+    note_text = "note text"
+    note = Notes(session, playlist.id, 0, note_text)
+    assert note
+
+    notes = session.query(Notes).all()
+    assert len(notes) == 1
+    assert notes[0].note == note_text
+
+    note.delete_note(session)
+    notes = session.query(Notes).all()
+    assert len(notes) == 0
+
+
+def test_notes_update_row_only(session):
+    # We need a playlist
+    playlist = Playlists(session, "my playlist")
+
+    note_text = "note text"
+    note = Notes(session, playlist.id, 0, note_text)
+    new_row = 10
+
+    note.update_note(session, new_row)
+
+    notes = session.query(Notes).all()
+    assert len(notes) == 1
+    assert notes[0].row == new_row
+
+
+def test_notes_update_text(session):
+    # We need a playlist
+    playlist = Playlists(session, "my playlist")
+
+    note_text = "note text"
+    note = Notes(session, playlist.id, 0, note_text)
+    new_text = "This is new"
+    new_row = 0
+
+    note.update_note(session, new_row, new_text)
+    notes = session.query(Notes).all()
+
+    assert len(notes) == 1
+    assert notes[0].note == new_text
+    assert notes[0].row == new_row
 
 
 def test_playdates_add_playdate(session):
@@ -48,14 +120,12 @@ def test_playdates_add_playdate(session):
 
     # We need a track
     track_path = "/a/b/c"
-    track = Tracks.get_or_create(session, track_path)
-    # Need to commit because track record is updated in Playdates.add_playdate()
-    session.commit()
+    track = Tracks(session, track_path)
 
-    playdate = Playdates.add_playdate(session, track)
+    playdate = Playdates(session, track)
+    assert playdate
 
     last_played = Playdates.last_played(session, track.id)
-
     assert playdate.lastplayed == last_played
 
 
@@ -64,78 +134,45 @@ def test_playdates_remove_track(session):
 
     # We need a track
     track_path = "/a/b/c"
-    track = Tracks.get_or_create(session, track_path)
-    # Need to commit because track record is updated in Playdates.add_playdate()
-    session.commit()
+    track = Tracks(session, track_path)
 
-    Playdates.add_playdate(session, track)
+    playdate = Playdates(session, track)
     Playdates.remove_track(session, track.id)
 
     last_played = Playdates.last_played(session, track.id)
-
     assert last_played is None
 
 
-def test_playlist_close(session):
-    """Test closing a playlist"""
+def test_playlist_create(session):
 
-    playlist = Playlists()
-    playlist.name = "Test playlist one"
-    session.add(playlist)
-    session.commit()
-
-    playlist2 = Playlists()
-    playlist2.name = "Test playlist two"
-    session.add(playlist2)
-    session.commit()
-
-    apl = Playlists.get_all(session)
-    assert len(apl) == 2
-
-    cpl = Playlists.get_closed(session)
-    assert len(cpl) == 0
-
-    playlist2.close(session)
-
-    cpl = Playlists.get_closed(session)
-    assert len(cpl) == 1
+    playlist = Playlists(session, "my playlist")
+    assert playlist
 
 
-def test_playlist_get_last_user(session):
-    """Test get_last_used function"""
+def test_playlist_add_note(session):
+    note_text = "my note"
+    note_row = 2
 
-    playlist1 = Playlists()
-    playlist1.name = "Test playlist one"
-    session.add(playlist1)
-    session.commit()
+    playlist = Playlists(session, "my playlist")
+    note = playlist.add_note(session, note_row, note_text)
 
-    playlist2 = Playlists()
-    playlist2.name = "Test playlist two"
-    session.add(playlist2)
-    session.commit()
-
-    playlist3 = Playlists()
-    playlist3.name = "Test playlist three"
-    session.add(playlist3)
-    session.commit()
-
-    apl = Playlists.get_all(session)
-    assert len(apl) == 3
-
-    playlist1.mark_open(session)
-    playlist2.close(session)
-    time.sleep(1)
-    playlist3.mark_open(session)
-
-    last_used = Playlists.get_open(session)
-    assert len(last_used) == 2
-    assert last_used[0].name == "Test playlist three"
+    assert len(playlist.notes) == 1
+    playlist_note = playlist.notes[0]
+    assert playlist_note.note == note_text
 
 
-def test_playlist_new(session):
-    """Test new function"""
+def test_playlist_add_track(session):
 
-    plname = "This is a test name"
-    p = Playlists(session, plname)
-    n = Playlists.get_by_id(session, p.id)
-    assert p.name == n.name
+    # We need a playlist
+    playlist = Playlists(session, "my playlist")
+
+    # We need a track
+    track_path = "/a/b/c"
+    track = Tracks(session, track_path)
+
+    playlist.add_track(session, track)
+
+    assert len(playlist.tracks) == 1
+    print(playlist.tracks)
+    playlist_track = playlist.tracks[0]
+    assert playlist_track.path == track_path