Review Request: Bibliography database UI

[Gopalakrishna Bhat]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105240/#review15049
-----------------------------------------------------------


I tested your branch today. First of all thanks for the nice work.

There are some issues that I found while testing your branch.


*) In the Bibliography Database dialog, if after pressing Crtl+N and then I cancel the dialog Words crashes.
*) If I open the same table thrice then in the Table selection drop down it is shown thrice
*) On changing the database table either the search box text should be cleared/the results should be filtered for the new table.
*) I feel that the option to change the search input types (RegEx/Fixed String/WildCards) should not be hidden inside the menu but rather shown below the search input box.
*) There is no option to remove filter conditions
*) Keeping the end user in mind the criteria for the filter condition can be a textual description rather that SQL operators (eg. "Not equal to" instead of "<>")
*) A behaviour similar to my 3rd point above for filters. Either the filters should be cleared on table change or the filters should be applied to the new table.
*) Data loss - While creating a new database table if the new database name is same as an existing .sqlite file then there should be a confirmation message instead of silently overwriting the original file
*) Do not open *.sqllite files that do not contain the bibref table
*) I get a lot of "QSqlDatabasePrivate::addDatabase: duplicate connection name 'qt_sql_default_connection', old connection removed." debug messages in the console please do remove it.


libs/kotext/KoInlineCite.cpp
<http://git.reviewboard.kde.org/r/105240/#comment11833>

    I did not get what are you refering to when you say that KoXmlWriter::addAttribute doesn't accept  attribute name in QString? My comment was on using loop insted of all these if conditions



plugins/textshape/dialogs/BibliographyDb.cpp
<http://git.reviewboard.kde.org/r/105240/#comment11834>

    hmm...I'm not sure you got my comment here. All the columns I see here are of type TEXT which should make my comment valid. Feel free to ping me on IRC if you want clarification on any of my commments



libs/kotext/BibliographyGenerator.cpp
<http://git.reviewboard.kde.org/r/105240/#comment11832>

    Please start the else in the same line as the close brace
    } else if() {



plugins/textshape/dialogs/EditFiltersDialog.cpp
<http://git.reviewboard.kde.org/r/105240/#comment11835>

    Instead of hard coding value 62 use insertWidget. If the pre-operator is not there there will be no widget in the first column.
    if (hasPreCondition) {
    m_layout->insertWidget(0, m_preCond);
    }
    
    m_layout->insertWidget(1, m_field);
    ...so on


- Gopalakrishna Bhat


On June 16, 2012, 4:28 p.m., Smit Patel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105240/
> -----------------------------------------------------------
> 
> (Updated June 16, 2012, 4:28 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> Bibliography database UI.
> 1) create/load/save/edit table saved in SQLite file
> 2) Search in database
> 3) Apply custom filters to database
> 4) Integrated this UI with existing insert citation for edit/create cite in database
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt 33cdabb 
>   libs/kotext/BibliographyGenerator.cpp 15808cb 
>   libs/kotext/KoInlineCite.h df4a2e9 
>   libs/kotext/KoInlineCite.cpp 6d949da 
>   libs/kotext/KoInlineTextObjectManager.cpp e82664d 
>   libs/kotext/KoTextEditor.cpp b3e16ba 
>   plugins/chartshape/TODO 94efb62 
>   plugins/textshape/CMakeLists.txt 6253323 
>   plugins/textshape/ReferencesTool.h 491b933 
>   plugins/textshape/ReferencesTool.cpp 564b658 
>   plugins/textshape/dialogs/BibliographyDatabaseWindow.h PRE-CREATION 
>   plugins/textshape/dialogs/BibliographyDatabaseWindow.cpp PRE-CREATION 
>   plugins/textshape/dialogs/BibliographyDatabaseWindow.ui PRE-CREATION 
>   plugins/textshape/dialogs/BibliographyDb.h PRE-CREATION 
>   plugins/textshape/dialogs/BibliographyDb.cpp PRE-CREATION 
>   plugins/textshape/dialogs/CitationInsertionDialog.h f5eac0b6 
>   plugins/textshape/dialogs/CitationInsertionDialog.cpp 14f9429 
>   plugins/textshape/dialogs/CitationInsertionDialog.ui 8f10f64 
>   plugins/textshape/dialogs/EditFiltersDialog.h PRE-CREATION 
>   plugins/textshape/dialogs/EditFiltersDialog.cpp PRE-CREATION 
>   plugins/textshape/dialogs/InsertCitationDialog.h PRE-CREATION 
>   plugins/textshape/dialogs/InsertCitationDialog.cpp PRE-CREATION 
>   plugins/textshape/dialogs/InsertCitationDialog.ui PRE-CREATION 
>   plugins/textshape/dialogs/SimpleCitationBibliographyWidget.cpp 2d1c1de 
>   plugins/textshape/dialogs/SimpleCitationBibliographyWidget.ui 55699bb 
> 
> Diff: http://git.reviewboard.kde.org/r/105240/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Smit Patel
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20120623/3e40f16e/attachment.htm>