[office/tellico/4.2] /: Validate multiple ISBN values
Robby Stephenson <[email protected]>
| Newsgroups | gmane.comp.kde.cvs |
|---|---|
| Message-ID | <[email protected]> |
Git commit dd2a6ea4cb42ecd31f5296d73b04785ba8a82289 by Robby Stephenson. Committed on 03/08/2026 at 01:03. Pushed by rstephenson into branch '4.2'. Validate multiple ISBN values Side effect is to make CueCat Intermediate status only for starting with .C3 CCBUG: 521157 M +4 -0 ChangeLog M +8 -4 src/gui/linefieldwidget.cpp M +6 -6 src/tests/CMakeLists.txt M +107 -1 src/tests/isbntest.cpp M +4 -0 src/tests/isbntest.h M +100 -2 src/utils/isbnvalidator.cpp M +7 -0 src/utils/isbnvalidator.h M +4 -2 src/utils/upcvalidator.cpp https://invent.kde.org/office/tellico/-/commit/dd2a6ea4cb42ecd31f5296d73b04785ba8a82289 diff --git a/ChangeLog b/ChangeLog index 248671ecb..2cc38dcda 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +2026-08-02 Robby Stephenson <[email protected]> + + * Updated ISBN validation for multiple values. + 2026-07-31 Robby Stephenson <[email protected]> * Updated UPCItemDb data source to allow multiple search values. diff --git a/src/gui/linefieldwidget.cpp b/src/gui/linefieldwidget.cpp index c58a48ebd..18ab74e32 100644 --- a/src/gui/linefieldwidget.cpp +++ b/src/gui/linefieldwidget.cpp @@ -47,8 +47,10 @@ LineFieldWidget::LineFieldWidget(Tellico::Data::FieldPtr field_, QWidget* parent } // Bug 521157: only validate if not marked for multiple values - if(field_->name() == QLatin1String("isbn") && !field_->hasFlag(Data::Field::AllowMultiple)) { - m_lineEdit->setValidator(new ISBNValidator(this)); + if(field_->name() == QLatin1StringView("isbn")) { + auto val = new ISBNValidator(this); + val->setAllowMultiple(field_->hasFlag(Data::Field::AllowMultiple)); + m_lineEdit->setValidator(val); } } @@ -83,8 +85,10 @@ void LineFieldWidget::updateFieldHook(Tellico::Data::FieldPtr, Tellico::Data::Fi m_lineEdit->setCompletionObject(nullptr); } - if(newField_->hasFlag(Data::Field::AllowMultiple)) { - m_lineEdit->setValidator(nullptr); + if(newField_->name() == QLatin1StringView("isbn")) { + auto val = new ISBNValidator(this); + val->setAllowMultiple(newField_->hasFlag(Data::Field::AllowMultiple)); + m_lineEdit->setValidator(val); } } diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt index 6c7a82db0..81b11d818 100644 --- a/src/tests/CMakeLists.txt +++ b/src/tests/CMakeLists.txt @@ -22,19 +22,19 @@ ecm_add_test(completiontest.cpp ../fieldcompletion.cpp ../field.cpp ../fieldform LINK_LIBRARIES Qt6::Test KF6::Completion utils config ) -ecm_add_test(cuecattest.cpp +ecm_add_test(cuecattest.cpp ../fieldformat.cpp TEST_NAME cuecattest - LINK_LIBRARIES Qt6::Test utils + LINK_LIBRARIES Qt6::Test utils config ) -ecm_add_test(isbntest.cpp +ecm_add_test(isbntest.cpp ../fieldformat.cpp TEST_NAME isbntest - LINK_LIBRARIES Qt6::Test utils + LINK_LIBRARIES Qt6::Test utils config ) -ecm_add_test(lccntest.cpp +ecm_add_test(lccntest.cpp ../fieldformat.cpp TEST_NAME lccntest - LINK_LIBRARIES Qt6::Test utils + LINK_LIBRARIES Qt6::Test utils config ) ecm_add_test(lcctest.cpp ../field.cpp ../fieldformat.cpp ../tellico_debug.cpp diff --git a/src/tests/isbntest.cpp b/src/tests/isbntest.cpp index cc55dda41..1b7d0638a 100644 --- a/src/tests/isbntest.cpp +++ b/src/tests/isbntest.cpp @@ -205,6 +205,7 @@ void IsbnTest::testState_data() { QTest::addColumn<QString>("value"); QTest::addColumn<bool>("changedValue"); + QTest::newRow("f") << QValidator::Invalid << QL1("f") << false; QTest::newRow("0") << QValidator::Intermediate << QL1("0") << false; QTest::newRow("0-") << QValidator::Intermediate << QL1("0-") << false; QTest::newRow("0-3") << QValidator::Intermediate << QL1("0-3") << false; @@ -220,7 +221,6 @@ void IsbnTest::testState_data() { QTest::newRow("0-321-11358-") << QValidator::Acceptable << QL1("0-321-11358-") << true; QTest::newRow("0-321-11358-6") << QValidator::Acceptable << QL1("0-321-11358-6") << false; - QTest::newRow("0") << QValidator::Intermediate << QL1("0") << true; QTest::newRow("03") << QValidator::Intermediate << QL1("03") << true; QTest::newRow("032") << QValidator::Intermediate << QL1("032") << true; QTest::newRow("0321") << QValidator::Intermediate << QL1("0321") << true; @@ -256,4 +256,110 @@ void IsbnTest::testState_data() { QTest::newRow("978-0-470-14762") << QValidator::Acceptable << QL1("978-0-470-14762") << true; QTest::newRow("978-0-470-14762-") << QValidator::Acceptable << QL1("978-0-470-14762-") << true; QTest::newRow("978-0-470-14762-7") << QValidator::Acceptable << QL1("978-0-470-14762-7") << false; + // invalid with a semi-colon and multiple values not allowed + QTest::newRow("false multiple") << QValidator::Invalid << QL1("978-0-470-14762-7; 9") << false; +} + +void IsbnTest::testMultiple() { + QFETCH(QValidator::State, expectedState); + QFETCH(QString, value); + QFETCH(QString, newValue); + + int pos = value.length() - 1; + + Tellico::ISBNValidator val; + val.setAllowMultiple(true); + QValidator::State state = val.validate(value, pos); + QCOMPARE(state, expectedState); + QCOMPARE(value, newValue); +} + +void IsbnTest::testMultiple_data() { + QTest::addColumn<QValidator::State>("expectedState"); + QTest::addColumn<QString>("value"); + QTest::addColumn<QString>("newValue"); + + QTest::newRow("multiple01") << QValidator::Acceptable << QL1("978-0-470-14762-7") << QL1("978-0-470-14762-7"); + QTest::newRow("multiple02") << QValidator::Intermediate << QL1("978-0-470-14762-7;") << QL1("978-0-470-14762-7; "); + QTest::newRow("multiple03") << QValidator::Intermediate << QL1("978-0-470-14762-7; ") << QL1("978-0-470-14762-7; "); + QTest::newRow("multiple04") << QValidator::Intermediate << QL1("978-0-470-14762-7;9") << QL1("978-0-470-14762-7; 9"); + QTest::newRow("multiple05") << QValidator::Intermediate << QL1("978-0-470-14762-7; 9") << QL1("978-0-470-14762-7; 9"); + QTest::newRow("multiple06") << QValidator::Intermediate << QL1("978-0-470-14762-7; 9") << QL1("978-0-470-14762-7; 9"); + QTest::newRow("multiple07") << QValidator::Intermediate << QL1("0321113586;03211135") << QL1("0-321-11358-6; 0-321-1135"); + QTest::newRow("multiple08") << QValidator::Acceptable << QL1("0321113586;032111358") << QL1("0-321-11358-6; 0-321-11358-6"); + QTest::newRow("multiple09") << QValidator::Acceptable << QL1("0321113586;0321113586") << QL1("0-321-11358-6; 0-321-11358-6"); + QTest::newRow("multiple10") << QValidator::Acceptable << QL1("0321113586;0321113586;0321113586") << QL1("0-321-11358-6; 0-321-11358-6; 0-321-11358-6"); + QTest::newRow("multiple11") << QValidator::Invalid << QL1("f;0321113586") << QL1("f; 0-321-11358-6"); +} + +void IsbnTest::testPos() { + QFETCH(QString, value); + QFETCH(int, pos); + QFETCH(int, newPos); + QFETCH(bool, multiple); + + Tellico::ISBNValidator val; + val.setAllowMultiple(multiple); + val.validate(value, pos); + QCOMPARE(pos, newPos); +} + +void IsbnTest::testPos_data() { + QTest::addColumn<QString>("value"); + QTest::addColumn<int>("pos"); + QTest::addColumn<int>("newPos"); + QTest::addColumn<bool>("multiple"); + + QTest::newRow("pos01") << QL1("978") << 2 << 2 << false; + QTest::newRow("pos02") << QL1("9780") << 3 << 3 << false; + QTest::newRow("pos03") << QL1("9780") << 4 << 5 << false; // 978-0 + QTest::newRow("pos04") << QL1("97804") << 3 << 3 << false; + QTest::newRow("pos05") << QL1("97804") << 4 << 5 << false; // 978-04 + QTest::newRow("pos06") << QL1("97804") << 5 << 6 << false; // 978-04 + QTest::newRow("pos07") << QL1("97804") << 6 << 6 << false; // 978-04 + QTest::newRow("pos08") << QL1("978047") << 6 << 7 << false; // 978-047 + QTest::newRow("pos09") << QL1("978047") << 7 << 7 << false; // 978-047 + QTest::newRow("pos10") << QL1("0446600989") << 0 << 0 << false; + QTest::newRow("pos11") << QL1("0446600989") << 1 << 1 << false; // 0-446-60098-9 + QTest::newRow("pos12") << QL1("0446600989") << 2 << 3 << false; // 0-446-60098-9 + QTest::newRow("pos13") << QL1("0446600989") << 3 << 4 << false; // 0-446-60098-9 + QTest::newRow("pos14") << QL1("0446600989") << 4 << 5 << false; // 0-446-60098-9 + QTest::newRow("pos15") << QL1("0446600989") << 5 << 7 << false; // 0-446-60098-9 + QTest::newRow("pos16") << QL1("0446600989") << 6 << 8 << false; // 0-446-60098-9 + QTest::newRow("pos17") << QL1("0446600989") << 7 << 9 << false; // 0-446-60098-9 + QTest::newRow("pos18") << QL1("0446600989") << 8 << 10 << false; // 0-446-60098-9 + QTest::newRow("pos19") << QL1("0446600989") << 9 << 11 << false; // 0-446-60098-9 + QTest::newRow("pos20") << QL1("0446600989") << 10 << 13 << false; // 0-446-60098-9 + QTest::newRow("pos21") << QL1("9780940016750") << 0 << 0 << false; // 978-0-940016-75-0 + QTest::newRow("pos22") << QL1("9780940016750") << 1 << 1 << false; // 978-0-940016-75-0 + QTest::newRow("pos23") << QL1("9780940016750") << 2 << 2 << false; // 978-0-940016-75-0 + QTest::newRow("pos24") << QL1("9780940016750") << 3 << 3 << false; // 978-0-940016-75-0 + QTest::newRow("pos25") << QL1("9780940016750") << 4 << 5 << false; // 978-0-940016-75-0 + QTest::newRow("pos26") << QL1("9780940016750") << 5 << 7 << false; // 978-0-940016-75-0 + QTest::newRow("pos27") << QL1("9780940016750") << 6 << 8 << false; // 978-0-940016-75-0 + QTest::newRow("pos28") << QL1("9780940016750") << 7 << 9 << false; // 978-0-940016-75-0 + QTest::newRow("pos29") << QL1("9780940016750") << 8 << 10 << false; // 978-0-940016-75-0 + QTest::newRow("pos30") << QL1("9780940016750") << 9 << 11 << false; // 978-0-940016-75-0 + QTest::newRow("pos31") << QL1("9780940016750") << 10 << 12 << false; // 978-0-940016-75-0 + QTest::newRow("pos32") << QL1("9780940016750") << 11 << 14 << false; // 978-0-940016-75-0 + QTest::newRow("pos33") << QL1("9780940016750") << 12 << 15 << false; // 978-0-940016-75-0 + QTest::newRow("pos34") << QL1("9780940016750") << 13 << 17 << false; // 978-0-940016-75-0 + QTest::newRow("pos101") << QL1("0446600989;04466") << 0 << 0 << true; // 0-446-60098-9; 0-4466 + QTest::newRow("pos102") << QL1("0446600989;04466") << 1 << 1 << true; // 0-446-60098-9; 0-4466 + QTest::newRow("pos103") << QL1("0446600989;04466") << 2 << 3 << true; // 0-446-60098-9; 0-4466 + QTest::newRow("pos104") << QL1("0446600989;04466") << 3 << 4 << true; // 0-446-60098-9; 0-4466 + QTest::newRow("pos105") << QL1("0446600989;04466") << 4 << 5 << true; // 0-446-60098-9; 0-4466 + QTest::newRow("pos106") << QL1("0446600989;04466") << 5 << 7 << true; // 0-446-60098-9; 0-4466 + QTest::newRow("pos107") << QL1("0446600989;04466") << 6 << 8 << true; // 0-446-60098-9; 0-4466 + QTest::newRow("pos108") << QL1("0446600989;04466") << 7 << 9 << true; // 0-446-60098-9; 0-4466 + QTest::newRow("pos109") << QL1("0446600989;04466") << 8 << 10 << true; // 0-446-60098-9; 0-4466 + QTest::newRow("pos110") << QL1("0446600989;04466") << 9 << 11 << true; // 0-446-60098-9; 0-4466 + QTest::newRow("pos111") << QL1("0446600989;04466") << 10 << 13 << true; // 0-446-60098-9; 0-4466 + QTest::newRow("pos112") << QL1("0446600989;04466") << 11 << 15 << true; // 0-446-60098-9; 0-4466 + QTest::newRow("pos113") << QL1("0446600989;04466") << 12 << 16 << true; // 0-446-60098-9; 0-4466 + QTest::newRow("pos114") << QL1("0446600989;04466") << 13 << 18 << true; // 0-446-60098-9; 0-4466 + QTest::newRow("pos115") << QL1("0446600989;04466") << 14 << 19 << true; // 0-446-60098-9; 0-4466 + QTest::newRow("pos116") << QL1("0446600989;04466") << 15 << 20 << true; // 0-446-60098-9; 0-4466 + QTest::newRow("pos117") << QL1("0446600989;04466") << 16 << 22 << true; // 0-446-60098-9; 0-4466 + QTest::newRow("pos118") << QL1("0446600989;04466") << 17 << 22 << true; // 0-446-60098-9; 0-4466 } diff --git a/src/tests/isbntest.h b/src/tests/isbntest.h index 325300b4d..e8105fa64 100644 --- a/src/tests/isbntest.h +++ b/src/tests/isbntest.h @@ -44,6 +44,10 @@ private Q_SLOTS: void testListDifference_data(); void testState(); void testState_data(); + void testMultiple(); + void testMultiple_data(); + void testPos(); + void testPos_data(); }; #endif diff --git a/src/utils/isbnvalidator.cpp b/src/utils/isbnvalidator.cpp index d07ceaf19..1db4dc871 100644 --- a/src/utils/isbnvalidator.cpp +++ b/src/utils/isbnvalidator.cpp @@ -24,6 +24,7 @@ #include "isbnvalidator.h" #include "upcvalidator.h" +#include "../fieldformat.h" #include <QStringList> #include <QRegularExpression> @@ -100,10 +101,72 @@ QString ISBNValidator::cleanValue(QString isbn) { } ISBNValidator::ISBNValidator(QObject* parent_) - : QValidator(parent_) { + : QValidator(parent_), m_allowMultiple(false) { +} + +void ISBNValidator::setAllowMultiple(bool allow_) { + m_allowMultiple = allow_; +} + +bool ISBNValidator::allowMultiple() const { + return m_allowMultiple; } QValidator::State ISBNValidator::validate(QString& input_, int& pos_) const { + if(!m_allowMultiple || + !FieldFormat::delimiterRegularExpression().match(input_).hasMatch()) { + return validateSingle(input_, pos_); + } + + const auto rx = FieldFormat::delimiterRegularExpression(); + + // find where each of the delimiters start + QList<int> valueStarts{0}; + for(auto i = rx.globalMatch(input_); i.hasNext(); ) { + valueStarts.append(i.next().capturedEnd()); + } + int cursorIndex = 0; + while(cursorIndex + 1 < valueStarts.length() && + valueStarts.at(cursorIndex + 1) <= pos_) { + ++cursorIndex; + } + + const QStringList values = input_.split(FieldFormat::delimiterRegularExpression(), + Qt::KeepEmptyParts); + QStringList finalValues; + State finalState = Acceptable; + int newPos = pos_; + + for(int i = 0; i < values.length(); ++i) { + QString value = values.at(i); + int valuePos = qBound(0, pos_ - valueStarts.at(i), value.length()); + const State valueState = validateSingle(value, valuePos); + + if(valueState == Invalid) { + finalState = Invalid; + } else if(valueState == Intermediate && finalState == Acceptable) { + finalState = Intermediate; + } + + // if pos_ is within this value, update for new pos + // by creating intermediate string, using length, and adding value pos + if(i == cursorIndex) { + newPos = finalValues.join(FieldFormat::delimiterString()).length(); + if(!finalValues.isEmpty()) { + newPos += FieldFormat::delimiterString().length(); + } + newPos += valuePos; + } + + finalValues += value; + } + + input_ = finalValues.join(FieldFormat::delimiterString()); + pos_ = qBound(0, newPos, input_.length()); + return finalState; +} + +QValidator::State ISBNValidator::validateSingle(QString& input_, int& pos_) const { // check if it's a cuecat first State catState = CueCat::decode(input_); if(catState != Invalid) { @@ -111,6 +174,11 @@ QValidator::State ISBNValidator::validate(QString& input_, int& pos_) const { return catState; } + static const QRegularExpression badChars(QStringLiteral("[^\\d\\-xX]")); + if(input_.contains(badChars)) { + return Invalid; + } + if(input_.startsWith(QLatin1StringView("978")) || input_.startsWith(QLatin1StringView("979"))) { return validate13(input_, pos_); @@ -152,7 +220,7 @@ QValidator::State ISBNValidator::validate10(QString& input_, int& pos_) const { } // remember if the cursor is at the end - bool atEnd = (pos_ == static_cast<int>(len)); + bool atEnd = (pos_ == len); // fix the case where the user attempts to delete a character from a non-checksum // position; the solution is to delete the checksum, but only if it's X @@ -170,10 +238,25 @@ QValidator::State ISBNValidator::validate10(QString& input_, int& pos_) const { } // now fixup the hyphens and maybe add a checksum + const QString oldInput = input_; fixup10(input_); len = input_.length(); // might have changed in fixup() if(atEnd) { pos_ = len; + } else if(input_.length() != oldInput.length()) { + int delta = 0; + for(int i = 0; i <= pos_; ++i) { + if(input_.length() > i+delta && + input_.at(i+delta) != oldInput.at(i)) { + ++delta; + } + } + // shift backwards if pos is now after a '-' + if(delta > 0 && (pos_+delta-1) < len && + input_.at(pos_+delta-1) == QLatin1Char('-')) { + --delta; + } + if(pos_ < len) pos_ += delta; } // first check to see if it's a "perfect" ISBN @@ -223,6 +306,7 @@ QValidator::State ISBNValidator::validate13(QString& input_, int& pos_) const { } // now fixup the hyphens and maybe add a checksum + const QString oldInput = input_; if(countN > 10) { fixup13(input_); } else { @@ -232,6 +316,20 @@ QValidator::State ISBNValidator::validate13(QString& input_, int& pos_) const { len = input_.length(); // might have changed in fixup() if(atEnd) { pos_ = len; + } else if(input_.length() != oldInput.length()) { + int delta = 0; + for(int i = 0; i <= pos_; ++i) { + if(input_.length() > i+delta && + input_.at(i+delta) != oldInput.at(i)) { + ++delta; + } + } + // shift backwards if pos is now after a '-' + if(delta > 0 && (pos_+delta-1) < len && + input_.at(pos_+delta-1) == QLatin1Char('-')) { + --delta; + } + if(pos_ < len) pos_ += delta; } // first check to see if it's a "perfect" ISBN13 diff --git a/src/utils/isbnvalidator.h b/src/utils/isbnvalidator.h index e2bc4f0a6..7bab84dba 100644 --- a/src/utils/isbnvalidator.h +++ b/src/utils/isbnvalidator.h @@ -42,6 +42,10 @@ Q_OBJECT public: ISBNValidator(QObject* parent = nullptr); + // whether to allow multiple values + void setAllowMultiple(bool allow); + bool allowMultiple() const; + /** * Certain conditions are checked. Character, length and position * restrictions are checked. Certain cases where the user is deleting @@ -145,6 +149,7 @@ private: int Last; } bands[]; + QValidator::State validateSingle(QString& input, int& pos) const; QValidator::State validate10(QString& input, int& pos) const; QValidator::State validate13(QString& input, int& pos) const; @@ -159,6 +164,8 @@ private: */ static QChar checkSum10(const QString& input); static QChar checkSum13(const QString& input); + + bool m_allowMultiple; }; class ISBNComparison { diff --git a/src/utils/upcvalidator.cpp b/src/utils/upcvalidator.cpp index 54ba456ce..d2a9edfe8 100644 --- a/src/utils/upcvalidator.cpp +++ b/src/utils/upcvalidator.cpp @@ -98,10 +98,12 @@ void UPCValidator::fixup(QString& input_) const { } QValidator::State Tellico::CueCat::decode(QString& input_) { - if(input_.length() < 3) { + if(input_.isEmpty() || + input_ == QLatin1StringView(".") || + input_ == QLatin1StringView(".C")) { return QValidator::Intermediate; } - if(!input_.startsWith(QLatin1String(".C3"))) { // all cuecat codes start with .C3 + if(!input_.startsWith(QLatin1String(".C3"))) { // all cuecat codes start with .C3 return QValidator::Invalid; } const int periods = input_.count(QLatin1Char('.'));