openlp
/
openlp
mirror of https://gitlab.com/openlp/openlp.git
1
0
Fork 1
Code Issues Projects Releases Wiki Activity

Fix bug in _has_header

This commit is contained in:
Gbenga Adeyi 2023-12-05 17:45:22 +00:00 committed by Raoul Snyman
parent 9844915164
commit cab39743e4
2 changed files with 40 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
openlp/plugins/bibles/lib/importers/csvbible.py
View File

@ -48,9 +48,12 @@ There are two acceptable formats of the verses file. They are:
All CSV files are expected to use a comma (',') as the delimiter and double quotes ('"') as the quote symbol.
"""
from enum import Enum
import logging
from collections import namedtuple
from csv import Error as CSVError, reader
from typing import Type, Union, Optional
from pathlib import Path
from openlp.core.common import get_file_encoding
from openlp.core.common.i18n import translate
@ -63,7 +66,12 @@ Book = namedtuple('Book', 'id, testament_id, name, abbreviation')
Verse = namedtuple('Verse', 'book_id_name, chapter_number, number, text')
def _has_header(sample):
class CSVBibleFileType(str, Enum):
Book = 'book'
Verse = 'verse'
def _has_header(sample: str, file_type: CSVBibleFileType) -> bool:
"""Determine if the sample of a csv file has a header line"""
if '\r\n' in sample:
lines = sample.split('\r\n')
@ -71,7 +79,13 @@ def _has_header(sample):
lines = sample.split('\n')
row_1 = lines[0].split(',')
row_2 = lines[1].split(',')
if all([row_2[0].isdigit(), row_2[1].isdigit()]) and not all([row_1[0].isdigit(), row_1[1].isdigit()]):
if file_type == CSVBibleFileType.Book and all([row_2[0].isdigit(),
row_2[1].isdigit()]) and not all([row_1[0].isdigit(),
row_1[1].isdigit()]):
return True
elif file_type == CSVBibleFileType.Verse and all([row_2[1].isdigit(),
row_2[2].isdigit()]) and not all([row_1[1].isdigit(),
row_1[2].isdigit()]):
return True
return False
@ -106,12 +120,17 @@ class CSVBible(BibleImport):
return book_name
@staticmethod
def parse_csv_file(file_path, results_tuple):
def parse_csv_file(
file_path: Path,
results_tuple: Type[Union[Book, Verse]],
file_type: CSVBibleFileType
) -> list[Union[Book, Verse]]:
"""
Parse the supplied CSV file.
:param pathlib.Path file_path: The name of the file to parse.
:param namedtuple results_tuple: The namedtuple to use to store the results.
:param CSVBibleFileType file_type: Specifies if its a book file or verses file.
:return: An list of namedtuples of type results_tuple
:rtype: list[namedtuple]
"""
@ -124,7 +143,7 @@ class CSVBible(BibleImport):
# Create the reader
csv_reader = reader(csv_file, delimiter=',', quotechar='"')
# Determine if the CSV has a header and skip if necessary
if _has_header(sample):
if _has_header(sample, file_type):
print("has_header")
next(csv_reader)
return [results_tuple(*line) for line in csv_reader]
@ -132,7 +151,7 @@ class CSVBible(BibleImport):
log.exception('Parsing {file} failed.'.format(file=file_path))
raise ValidationError(msg='Parsing "{file}" failed'.format(file=file_path))
def process_books(self, books):
def process_books(self, books: list[Book]) -> dict:
"""
Process the books parsed from the books file.
@ -150,7 +169,7 @@ class CSVBible(BibleImport):
book_list.update({int(book.id): book.name})
return book_list
def process_verses(self, verses, books):
def process_verses(self, verses: list[Verse], books: dict):
"""
Process the verses parsed from the verses file.
@ -173,7 +192,7 @@ class CSVBible(BibleImport):
self.create_verse(book.id, int(verse.chapter_number), int(verse.number), verse.text)
self.session.commit()
def do_import(self, bible_name=None):
def do_import(self, bible_name: Optional[str] = None):
"""
Import a bible from the CSV files.
@ -183,12 +202,12 @@ class CSVBible(BibleImport):
self.language_id = self.get_language(bible_name)
if not self.language_id:
return False
books = self.parse_csv_file(self.books_path, Book)
books: list[Book] = self.parse_csv_file(self.books_path, Book, CSVBibleFileType.Book)
self.wizard.progress_bar.setValue(0)
self.wizard.progress_bar.setMinimum(0)
self.wizard.progress_bar.setMaximum(len(books))
book_list = self.process_books(books)
verses = self.parse_csv_file(self.verses_path, Verse)
verses: list[Verse] = self.parse_csv_file(self.verses_path, Verse, CSVBibleFileType.Verse)
self.wizard.progress_bar.setValue(0)
self.wizard.progress_bar.setMaximum(len(books) + 1)
self.process_verses(verses, book_list)

23
tests/openlp_plugins/bibles/test_csvimport.py
View File

@ -29,7 +29,7 @@ from unittest.mock import MagicMock, PropertyMock, call, patch
from openlp.core.lib.exceptions import ValidationError
from openlp.plugins.bibles.lib.bibleimport import BibleImport
from openlp.plugins.bibles.lib.importers.csvbible import Book, CSVBible, Verse, _has_header
from openlp.plugins.bibles.lib.importers.csvbible import Book, CSVBible, CSVBibleFileType, Verse, _has_header
from tests.utils import load_external_result_data
from tests.utils.constants import RESOURCE_PATH
@ -121,8 +121,8 @@ def test_parse_csv_file():
Test the parse_csv_file() with sample data
"""
# GIVEN: A mocked csv.reader which returns an iterator with test data
test_data = [['1', 'Line 1', 'Data 1'], ['2', 'Line 2', 'Data 2'], ['3', 'Line 3', 'Data 3']]
TestTuple = namedtuple('TestTuple', 'line_no line_description line_data')
test_data = [['1', '1', 'name 1', 'abbrev 1'], ['2', '2', 'name 2', 'abbrev 2'], ['3', '3', 'name 3', 'abbrev 3']]
TestTuple = namedtuple('TestTuple', 'line_no line_id name abbrev')
mocked_csv_file = MagicMock()
mocked_enter_file = MagicMock()
mocked_csv_file.open.return_value.__enter__.return_value = mocked_enter_file
@ -134,11 +134,11 @@ def test_parse_csv_file():
mocked_has_header.return_value = False
# WHEN: Calling the CSVBible parse_csv_file method with a file name and TestTuple
result = CSVBible.parse_csv_file(mocked_csv_file, TestTuple)
result = CSVBible.parse_csv_file(mocked_csv_file, TestTuple, CSVBibleFileType.Book)
# THEN: A list of TestTuple instances with the parsed data should be returned
assert result == [TestTuple('1', 'Line 1', 'Data 1'), TestTuple('2', 'Line 2', 'Data 2'),
TestTuple('3', 'Line 3', 'Data 3')]
assert result == [TestTuple('1', '1', 'name 1', 'abbrev 1'), TestTuple('2', '2', 'name 2', 'abbrev 2'),
TestTuple('3', '3', 'name 3', 'abbrev 3')]
mocked_csv_file.open.assert_called_once_with('r', encoding='utf-8', newline='')
mocked_reader.assert_called_once_with(mocked_enter_file, delimiter=',', quotechar='"')
@ -153,7 +153,7 @@ def test_has_header():
"""
# WHEN: Sample data is given to _has_header()
result = _has_header(test_data)
result = _has_header(test_data, CSVBibleFileType.Book)
# THEN: The result should be true
assert result is True
@ -169,7 +169,7 @@ def test_has_no_header():
"""
# WHEN: Sample data is given to _has_header()
result = _has_header(test_data)
result = _has_header(test_data, CSVBibleFileType.Book)
# THEN: The result should be true
assert result is False
@ -190,7 +190,7 @@ def test_parse_csv_file_oserror():
# WHEN: Calling CSVBible.parse_csv_file
# THEN: A ValidationError should be raised
with pytest.raises(ValidationError) as context:
CSVBible.parse_csv_file(mocked_csv_file, None)
CSVBible.parse_csv_file(mocked_csv_file, None, CSVBibleFileType.Book)
assert context.value != ValidationError('Parsing "file.csv" failed')
@ -209,7 +209,7 @@ def test_parse_csv_file_csverror():
# WHEN: Calling CSVBible.parse_csv_file
# THEN: A ValidationError should be raised
with pytest.raises(ValidationError) as context:
CSVBible.parse_csv_file(mocked_csv_file, None)
CSVBible.parse_csv_file(mocked_csv_file, None, CSVBibleFileType.Book)
assert context.value != ValidationError('Parsing "file.csv" failed')
@ -359,7 +359,8 @@ def test_do_import_success(registry):
# THEN: parse_csv_file should be called twice,
# and True should be returned.
assert importer.parse_csv_file.mock_calls == \
[call(Path('books.csv'), Book), call(Path('verses.csv'), Verse)]
[call(Path('books.csv'), Book, CSVBibleFileType.Book),
call(Path('verses.csv'), Verse, CSVBibleFileType.Verse)]
importer.process_books.assert_called_once_with(['Book 1'])
importer.process_verses.assert_called_once_with(['Verse 1'], ['Book 1'])
assert result is True