Bug 49554 - Misleading error message about primary key on first page of table import wizard
Summary: Misleading error message about primary key on first page of table import wizard
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: Other All
: medium normal
Assignee: Muhammet Kara
URL:
Whiteboard: target:5.2.0
Keywords:
Depends on:
Blocks:
 
Reported: 2012-05-06 08:58 UTC by Regina Henschel
Modified: 2016-10-25 19:08 UTC (History)
6 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Regina Henschel 2012-05-06 08:58:05 UTC
When you import a Calc table to a database, for example with drag&drop to the table container, then the Wizard "Copy Table" starts.
On the first page you can check "Create Primary Key" and enter an identifier in the field 'Name'. If you enter a column name (for example 'Ident') of the Calc table, you get the error message "The following fields have already been set as primary keys: Ident"

This error message is misleading, because it says, that the column 'Ident' has already been set as primary key. But that is not the case. If you will use an existing column as primary key, you have to set it on the third page of that wizard.

The source code in dbaccess/source/ui/misc/WCPage.cxx has a correct commend "// now we have to check if the name of the primary key already exists" but uses the unsuitable string STR_WIZ_PKEY_ALREADY_DEFINED.

I think, that two improvements are possible and needed
(1)
Define a new string for the error message.
For example, "Enter a unique name for the new primary key data field. The following name is already in use: ..." 
(2)
Change the description in the wizard, so that the user does no longer thinks, he can make an existing columns to a primary key with this check box. For example, "Create new data field for primary key." instead of "Create primary key"
Comment 1 Robert Großkopf 2012-05-06 09:31:18 UTC
Better way could be:
(3) Change the wizard. Create the primary key at the first page. Looks like this:
0  Create primary key
      Name for new field: [(textfield)]
      Name of field of the table: [(listbox with fields)]

Creating the primary key of existing fields at the third page of the wizard and of new fileds at the first page of the wizard isn't very intuitive. Hiding the creating of a primary key for existing fields in the third page in a kontext-menue for only this command is a good way user may not find it.
Comment 2 Jochen 2012-05-06 09:51:06 UTC
Further information:
this problem was discussed in detail in early June 2012 on the German discuss-ML (see http://listarchives.libreoffice.org/de/discuss/msg12325.html).
Specialists are Regina and Robert.
Comment 3 Jochen 2012-05-06 09:54:31 UTC
correction:
... was discussed in detail in early May 2012 ...

Sorry.
Comment 4 Robert Großkopf 2012-07-09 15:16:53 UTC
*** Bug 51903 has been marked as a duplicate of this bug. ***
Comment 5 Ulf Zibis 2012-07-19 19:30:01 UTC
(In reply to comment #1)
> 0  Create primary key
>       Name for new field: [(textfield)]
>       Name of field of the table: [(listbox with fields)]

I think, customizing the properties of existing fields should be provided exclusively on the 3rd wizard page. Have in mind, that sometimes more than 1 column is to be used for primary key, and most likely VARCHAR is preset for existing fields, which should not fit for primary key in most cases.

So I would suggest for the 1st page:

[ ] Create additional primary key column
    Name [        ]   (existing columns can be customized on 3rd wizard page)

> Hiding the
> creating of a primary key for existing fields in the third page in a
> kontext-menue for only this command is a good way user may not find it.

+1
See alternative from attachment 64014 [details] at bug 51903.

(In reply to comment #0)
> (1)
> Define a new string for the error message.
> For example, "Enter a unique name for the new primary key data field. The
> following name is already in use: ..."

+1, but add (existing columns can be set as primary key on 3rd wizard page)
Comment 6 Alex Thurgood 2015-01-03 17:40:49 UTC Comment hidden (noise)
Comment 7 QA Administrators 2016-01-17 20:04:01 UTC Comment hidden (noise)
Comment 8 Muhammet Kara 2016-04-10 16:51:58 UTC
I confirm that the issue still exists as of LibreOffice 5.2.0.0.alpha0 on Linux x86_64.

And I have started working on fixing it.

I think I will fix the misleading error message first. Since it is not used elsewhere, I'll just modify the existing string as "Enter a unique name for the new primary key data field.\nThe following name is already in use:".

And I'll change "Create primary key" on the first page of the wizard to "Create additional primary key" to point out that it will be an additional field other than the "will-be" imported fields. (Or "Create new field as primary key"?)

Then I'll add a checkbox or button named something like "Set as primary key" on the third page of the wizard.

What do you think?
Comment 9 Regina Henschel 2016-04-10 17:28:23 UTC
(In reply to Muhammet Kara from comment #8)
> I confirm that the issue still exists as of LibreOffice 5.2.0.0.alpha0 on
> Linux x86_64.
> 
> And I have started working on fixing it.

Nice that there is some movement after 4 years :)

> 
> I think I will fix the misleading error message first. Since it is not used
> elsewhere, I'll just modify the existing string as "Enter a unique name for
> the new primary key data field.\nThe following name is already in use:".
> 
> And I'll change "Create primary key" on the first page of the wizard to
> "Create additional primary key" to point out that it will be an additional
> field other than the "will-be" imported fields. (Or "Create new field as
> primary key"?)


"Create additional primary key" is wrong, because a primary key is always unique and therefore there cannot be an 'additional' one. Therefore your suggestion "Create new field as primary key" is much better.

In addition the user needs an information on the first page, that he cannot make any of the existing fields to a primary key on the first page of the wizard, but using an exiting field is only possible on the third page of the wizard.

> 
> Then I'll add a checkbox or button named something like "Set as primary key"
> on the third page of the wizard.
> 
> What do you think?

A checkbox will not work, because you need a way to make a primary key, which consists of several fields.
Comment 10 Muhammet Kara 2016-04-10 19:43:38 UTC
(In reply to Regina Henschel from comment #9)

I have submitted a patch for this bug. Comments are welcome. :)

https://gerrit.libreoffice.org/#/c/23969/

And I guess if this patch is pushed (as it is or after some adjusting), we can consider this bug as resolved-fixed?

Should we leave it open for the other suggested changes (about the way how primary key is set with existing fields) or let that suggestion live in another bug report?
Comment 11 Ulf Zibis 2016-04-10 23:20:37 UTC
(In reply to Regina Henschel from comment #9)
> A checkbox will not work, because you need a way to make a primary key,
> which consists of several fields.
I do not understand your concern. For each data field, the box could be checked separately, so multi-selection is possible with suggestion from attachment 64014 [details] at bug 51903.

(In reply to Muhammet Kara from comment #10)
> https://gerrit.libreoffice.org/#/c/23969/
The name STR_WIZ_PKEY_ALREADY_DEFINED would be better changed to STR_WIZ_NAME_ALREADY_DEFINED.
Why you use the plural form, isn't there only 1 name possible to enter?
I think you could drop "right click on the field names"-part in the label, otherwise you have to change it again when the bug 51903 part is fixed.
Otherwise the wording looks good to me.

> ... or let that suggestion live in another bug report?
You could use bug 51903 for that, just by un-marking the duplicate state, which anyway seems wrong to me.
Comment 12 Muhammet Kara 2016-04-11 07:33:31 UTC
(In reply to Ulf Zibis from comment #11)
> 
> (In reply to Muhammet Kara from comment #10)
> > https://gerrit.libreoffice.org/#/c/23969/
> The name STR_WIZ_PKEY_ALREADY_DEFINED would be better changed to
> STR_WIZ_NAME_ALREADY_DEFINED.
> Why you use the plural form, isn't there only 1 name possible to enter?

I did so because a primary key may consist of several fields, and I assumed that it is possible to enter several field names there. (I was wrong)

Even if I enter several names separated with comma, it considers them as one. For example:
If my copied data has two fields named "Name" and "Surname". If I enter one of them into this box, it gives the error; but if I enter both of them together as "Name,Surname", it creates a single primary key field named as "Name,Surname". I don't know if we should open a bug report for this behaviour.

> I think you could drop "right click on the field names"-part in the label,
> otherwise you have to change it again when the bug 51903 part is fixed.
> Otherwise the wording looks good to me.

Will do that. Thanks for the warning.

> 
> > ... or let that suggestion live in another bug report?
> You could use bug 51903 for that, just by un-marking the duplicate state,
> which anyway seems wrong to me.

I'll probably do that. Consulting on #libreoffice-qa right now.
Comment 13 Lionel Elie Mamane 2016-04-11 08:03:12 UTC
(In reply to robert from comment #1)
> Better way could be:
> (3) Change the wizard. Create the primary key at the first page. Looks like
> this:
> 0  Create primary key
>       Name for new field: [(textfield)]
>       Name of field of the table: [(listbox with fields)]

On the first page, the fields that will be included are not chosen yet. So one cannot choose which ones will be part of the primary key, since it is unknown which ones will be there. I'd rather take inspiration from the "new table" wizard which presents the choices in logical order and the several alternatives clearly.

Muhammet: if you feel like working in this bigger project, then all the better, else I'll gladly merge a patch for better error message and/or explanatory label which is already positive progress.
Comment 14 Muhammet Kara 2016-04-11 10:19:42 UTC
(In reply to Lionel Elie Mamane from comment #13)
> (In reply to robert from comment #1)
> > Better way could be:
> > (3) Change the wizard. Create the primary key at the first page. Looks like
> > this:
> > 0  Create primary key
> >       Name for new field: [(textfield)]
> >       Name of field of the table: [(listbox with fields)]
> 
> On the first page, the fields that will be included are not chosen yet. So
> one cannot choose which ones will be part of the primary key, since it is
> unknown which ones will be there. I'd rather take inspiration from the "new
> table" wizard which presents the choices in logical order and the several
> alternatives clearly.
> 
> Muhammet: if you feel like working in this bigger project, then all the
> better, else I'll gladly merge a patch for better error message and/or
> explanatory label which is already positive progress.

I have adjusted the patch and resubmitted to gerrit.[0] I would like to close this bug as fixed, and continue working on the wizard (via tdf#51903) if that's Ok. (As the initial purpose of the bug suggested by its title and description is fixing the misleading error message.)


[0] https://gerrit.libreoffice.org/#/c/23969/
Comment 15 Commit Notification 2016-04-11 12:00:19 UTC
Muhammet Kara committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=87e949b68e9931ed5f19ab6ceb8a709ec2d3d6cf

tdf#49554 Fix misleading error message

It will be available in 5.2.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 16 Adolfo Jayme Barrientos 2016-04-13 00:41:59 UTC
> I would like to close this bug as fixed, and continue working on the wizard
> (via tdf#51903)

That sounds good, let’s continue on bug 51903. Thanks for your efforts, Muhammet!