unit test and refactor database.py part 2

[SRomansky]

Add unit tests and refactor/fix/document issues related to the database.
(Part 2, because #619 grew too large.)

Related #484
Related #619

[SRomansky]

@dideler @zxiiro
https://gist.github.com/wigglier/9591f5d05d4752f65d41 this might be the culprit for why the database appears to have NULLs instead of empty-string characters.

edit: This is from adding a print statement to insert_presentation()

[SRomansky]

    'Title,Speaker,Abstract,Category,Event,Room,Date,StartTime,EndTime\r\n',
        'Managing map data in a database,Andrew Ross,,,Default,Default,,,\r\n',
        'Building NetBSD,David Maxwell,,,Default,Default,,,\r\n',
        'Faking it till you make it,John Doe,,,Default,Default,,,\r\n'

From the expected csv lines. Note that the ,, is like None pretty much. So SQL isn't inserting '' at all. instead it is taking the content from the string objects (which is nothing) and inserting it into the prepared presentation fields in database.py. Hence, we get None in the db.

[SRomansky]

    def _helper_presentation_exists(self, presentation):
        """This is used in testing. Checks if there's a presentation with the same Speaker and Title already stored."""
        query = QtSql.QSqlQuery()

        query.prepare('''                                                                                             
            SELECT *                                                                                                  
            FROM presentations                                                                                        
            WHERE Title=:title AND Speaker=:speaker AND Description=:description AND Category=:category AND           
                  Event=:event AND (Room is NULL OR Room=:room) AND (Date=:date OR Date is NULL) AND                  
                  (StartTime=:startTime or StartTime is Null) AND EndTime=:endTime                                    
        ''')
        query.bindValue(':title', presentation.title)
        query.bindValue(':speaker', presentation.speaker)
        query.bindValue(':description', presentation.description)
        query.bindValue(':category', presentation.category)
        query.bindValue(':event', presentation.event)
        query.bindValue(':room', presentation.room)
        query.bindValue(':date', presentation.date)
        query.bindValue(':startTime', presentation.startTime)
        query.bindValue(':endTime', presentation.endTime)
        query.exec_()
        print query.boundValue(':room').toString()

        if query.next():
            return True
        else:
            return False

This should be equivalent to what is already in database.py but it is not for some reason

Specifically when presentation.room='' this fails.

[dideler]

I'm confused by all this date/time stuff. You're now passing the time to the date field?

[SRomansky]

sqlite and python website mentions that python and SQLITE treat the timestamp SQL data type as a DateTime Python object.

The SQL statement on line 192 says to only take the date from the given timestamp value, date documentation.
However, SQL is kind of confusing. I changed it such that Python now get the date from the DateTime object.

In summary, the presentation.date and presentation.startTime or presentation.endTime are both of the type QDateTime. presentation.date has a more specific type QDate QDate which only contains the date part.
Both Python and SQLITE know how to convert QDateTime objects into QDate objects using a version of the date() call. In SQLITE it is just applied to a value with a timestamp like any function call in SQL, and in Python date() can be used on a QDateTime object.

edit:

Example:

# ./frontend/reporteditor/reporteditor.py
    def add_talk(self):
        date = self.addTalkWidget.dateEdit.date()
        startTime = self.addTalkWidget.startTimeEdit.time()
        datetime = QDateTime(date, startTime)  # original "time" is now "startTime"                                                                                                                                              
        presentation = Presentation(unicode(self.addTalkWidget.titleLineEdit.text()),
                                    unicode(self.addTalkWidget.presenterLineEdit.text()),
                                    "",  # description                                                                                                                                                                           
                                    "",  # level                                                                                                                                                                                 
                                    unicode(self.addTalkWidget.eventLineEdit.text()),
                                    unicode(self.addTalkWidget.roomLineEdit.text()),
                                    unicode(datetime.toString()),  # presentation.date
                                    unicode(self.addTalkWidget.endTimeEdit.text()))  # presentation.startTime

This code sets the presentation.date to DateTime instead of just a Date object. (Making some confusion..)

[dideler]

If I understand you correctly, the Date field (with type TIMESTAMP) can work with QDateTime or QDate? If so, I think being explicit and using QDate is better than implicitly converting a QDateTime into QDate.

If I misunderstand and that's not possible, then I think there should be an inline comment on L192, e.g. "Takes the date from the timestamp value."

[SRomansky]

QDate can work.

The complication is that the rest of the code needs to enforce this convention (even though it will work fine with the QDateTime objects too). I will start coding this now.

[SRomansky]

Hm. One issue I have caused is the date, starttime, endtime in the talkeditor table are now showing the funky timestamp values instead of human readable times..

[SRomansky]

@dideler https://www.sqlite.org/datatype3.html in section 1.2 it mentions that SQLITE does not have a DATE or a TIME type and instead just has that timestamp one. :(.

[dideler]

The first record uses the latest changes on this branch.
The second record is before the changes introduced in 43d892f.
The last record uses the code below.

def create_presentation(self, talkDetailsWidget):
    """Creates and returns an instance of Presentation using data from the input fields"""
    title = unicode(talkDetailsWidget.titleLineEdit.text()).strip()
    if title:
        return Presentation(
            title,
            unicode(talkDetailsWidget.presenterLineEdit.text()).strip(),
            unicode(talkDetailsWidget.descriptionTextEdit.toPlainText()).strip(),
            unicode(talkDetailsWidget.categoryLineEdit.text()).strip(),
            unicode(talkDetailsWidget.eventLineEdit.text()).strip(),
            unicode(talkDetailsWidget.roomLineEdit.text()).strip(),
            talkDetailsWidget.dateEdit.date(),
            talkDetailsWidget.startTimeEdit.time().toString("hh:mm ap"),
            talkDetailsWidget.endTimeEdit.time().toString("hh:mm ap"))

[dideler]

Exporting talks to CSV and then importing them gets rid of the date and start/end times. Haven't checked an earlier version to see if it worked, but if it did, this is a regression and will need to be fixed before merging.

[SRomansky]

@dideler the missing dates on import seems to occur in master too.

[SRomansky]

@dideler I think the csv cannot be imported because the exporter uses StartTime,EndTime in its output and the import utility looks for only a Time field.

[dideler]

@wigglier you asked on IRC if this should be split up into smaller commits.

I think it would help and shouldn't be too difficult. You can group files into new PRs, for example:

# Create new branch for new PR
git checkout -b refactor-rss-stuff-for-tests 484-unit-test-and-refactor-p2

# Uncommit the single squashed commit
git reset --soft HEAD^

# Unstage the files you want in the new PR
git reset src/freeseer/tests/plugins/importer/rss_feedparser/test_rss_feedparser.py \
    src/freeseer/tests/resources/sample_rss_data/sc2011.rss \
    src/freeseer/tests/resources/sample_rss_data/summercamp2010.json  \
    src/freeseer/tests/resources/sample_rss_data/summercamp2010.rss \
    src/freeseer/tests/plugins/importer/rss_feedparser/presentation_feed.json \
    src/freeseer/tests/plugins/importer/rss_feedparser/presentation_feed.rss

# Temporarily commit all the other changes
git commit -m 'WIP'

# Stage and commit all the RSS stuff 
git add .
git commit

# Remove the WIP commit so this branch only has the RSS changes commit
git rebase -i HEAD^^  # Just delete the line with the WIP commit

[SRomansky]

#679 created, @dideler

[SRomansky]

@dideler when #679 gets merged, I can remove the rss related files from this PR.
I can split the 1 giant commit into smaller pieces too, e.g. one commit for each file or two added to tests/framework/database

[SRomansky]

@dideler I removed the code that stored the time format as 'hh:mm ap' and just kept it as 'hh:mm:ss'.
The first problem from using 'hh:mm ap' is that the database stores a different value than what is in csv files and what the QTime object uses as its default time format. So there are extra function calls required to convert from hh:mm:ss -> hh:mm ap and back again.

The second problem is that I am not sure how the database is going to handle sorting timestamp values with entries like '3:52 pm' instead of '15:52:00'. In Python if the string '3:52 pm' is cast into the QTime object with out a format specified pyqt will just cast it to the null QTime object. Therefore, I am not sure how SQLITE will handle '3:52 pm' instead of the default time format.

[SRomansky]

L44-46 can be removed.

[dideler]

What about displaying the time in a human friendly format but saving it however is easiest in the DB?

[SRomansky]

@dideler I think that can be done. I will investigate how.