search

salesagility / SuiteCRM

SuiteCRM - Open source CRM for the world
https://www.suitecrm.com
GNU Affero General Public License v3.0
4.45k stars 2.08k forks source link

Record Duplicate button does not copy all fields #7915

Open communig8-public opened 5 years ago

communig8-public commented 5 years ago

Issue

When the Duplicate button is used to duplicate a record, only fields included on the EditView and are not readonly, are written to the new record.

Expected Behavior

All fields on the source record should be written to the new record.

Actual Behavior

Only those records that have been included on the EditView and are not readonly are written to the new record.

Steps to Reproduce

  1. View a record
  2. Click the Duplicate button
  3. Save the new record

Your Environment

Mac-Rae commented 5 years ago

I was unable to replicate this on the newest version. Could you try replication this on demo.suitecrm.com which runs the most up to date version and see if your issue is resolved

communig8-public commented 5 years ago

It cant be tested on the demo system as their is no admin access to alter an EditView.

pgorod commented 5 years ago

Demo with Admin access:

https://www.softaculous.com/demos/SuiteCRM

communig8-public commented 5 years ago

Tried it on 7.11.8 by creating an Account record with an Office Phone set, modified the Editview so Office Phone was not on the layout, duplicated it to a new record and the Office Phone was not set. See attached...

original duplicated
pgorod commented 5 years ago

Yes it seems the code is only working from the POST arguments in the Edit view. This does not include fields that are not in the view...

Maybe this has something to do with it: https://github.com/salesagility/SuiteCRM/blob/master/include/EditView/EditView2.php#L742-L748

I am not sure I understand why it's clearing the fields there...

communig8-public commented 5 years ago

So it is a bug then?

pgorod commented 5 years ago

Or at least a "feature that hadn't occurred to anybody before" :-)

communig8-public commented 5 years ago

Button says "Duplicate", the record is not duplicated (missing fields), it's a Bug!

pgorod commented 5 years ago

It doesn't say "Duplicate record" or "Duplicate Account". You can read it as "Duplicate view" or "Duplicate this that you're looking at". So I guess this is debatable.

But if we debate it, I will be on your side: I believe it's better to duplicate the entire record.

communig8-public commented 5 years ago

No, it's really simple, the button says "Duplicate", the record is not duplicated (missing fields), it's a Bug!

communig8-public commented 5 years ago

@Mac-Rae I dont understand why this is flagged as "Requires Update", what updates are required?

Dillon-Brown commented 5 years ago

Hi @communig8-public, no updates required since you've given plenty of detail. Hard to say whether this should be considered as a suggestion or a bug.... I think I'd lean more towards bug simply for clarity but it does look like the current functionality is working as intended even if it doesn't seem very logical.

communig8-public commented 5 years ago

@Dillon-Brown - Oxford English Dictionary "Duplicate - duplicate something to make an exact copy of something". We could change the button name I support to "Copy only those fields that are included on the EditView that are not ReadOnly" but heck, the button isn't even on the EditView.

Also, how can data loss be considered "Low Priority"?

Dillon-Brown commented 5 years ago

Hi @communig8-public, while I appreciate the Oxford Dictionary definition, I would argue strongly in favour of descriptivism when it comes to the English language.

Language is an ever-changing and developing expression of human personality, and does not grow well under rigorous direction. — C. L. Wrenn, The English Language

With that being said, I'll gladly go into detail with you over what we mean here when we use the term "Bug", "Suggestion" and "Low Priority" in the context of software engineering and more specifically the SalesAgility repositories.

Bug: "A confirmed error, flaw, or failure within the system that causes it to produce an incorrect or unexpected result".

Suggestion: "A suggestion to add new functionality to the system that was not part of the original design or specificiaton".

Low Priority: "An issue that has a relatively minor impact on most users with potential workarounds".

In this specific case, the original intention of this functionality was to duplicate the view and not the record itself, which in reality would make this a suggestion or a bug requiring a simple change to the label updating it to "Duplicate View" or something similar for clarity. However, I personally agree with you that it would make more sense to duplicate the actual record itself, instead of the view. This is why I think we can reasonably consider this a "Bug" instead of a new feature. At the same time, I wouldn't necessarily consider it higher than "Low Priority" since as previously stated, it's technically working as expected.

communig8-public commented 5 years ago

I really don't think the meaning of the word "Duplicate" has ever changed or is likely to. The "man in the street" would describe "Duplicate" as "make an exactly copy", which it is not doing. It's not even duplicating the "view" as the button is on the DetailView and what is "Duplicated" is governed by what is on the EditView so that interpretation does not stack up either. So I am glad that you agree is is a bug (putting aside the madness of "Working as Designed"). Now for the priority of a data loss bug, how could that be regarded as "minor impact" and what would the workaround be?

Dillon-Brown commented 5 years ago

@communig8-public Agreed the definition of "Duplicate" isn't likely to change, it should be an exact copy. What I think is debatable is here is what should it be an "exact copy" of? the record or the view? (Imo it should be the record). Also agreed that even if we were to say that it should be the view then it still wouldn't make much sense since it's not on the edit-view, either way, something should be changed here.

For the "minor impact", my thinking is that while not ideal, there's nothing preventing you from manually copying over the data you need. Furthermore, since it looks like this is the way that the "Duplicate" functionality has always worked, It shouldn't cause a users normal process to all of a sudden result in data loss due to expecting the "Duplicate" button to copy everything over like a bug introduced in a previous release might have done.

communig8-public commented 5 years ago

Perhaps it would help to give a specific example of a problem cased by the loss of data during a "Duplicate" operation.

I have a client that is a manufacturer. They create a physical mould for the production of parts. Usually one part is produced from one mould but in some cases more than one part could be created needing the same mould but say of a different colour. Each mould has a unique serial number that is used in the record for the part. They create a new mould and a work flow generates a unique serial number. The "blue" part has this serial number and they want to duplicate the part record to make a "yellow" part holding the same serial number as the "blue" part but they don't want anyone to change the serial number. So the DetailView of the part shows the serial number and the EditView does not. When they use the "Duplicate" button, the serial number is missing from the duplicated record.

communig8-public commented 5 years ago

OK I'm going to back off and leave you guys to ruminate over the ambiguity of the English language and the relative severity of data loss vs. having a cup of coffee. I'm using a before_save logic hook to correct for this bug so my client can get their work done.