[5.4] Fix: Banner clients accept duplicates

[harshkhatri8]

Pull Request for Issue #45603 Fixes #45603

Summary of Changes

-Updated ClientModel.php to prevent duplicate client names when using Save as Copy. -Implemented automatic unique name generation: if a client with the same name exists, the system appends (2), (3), etc. -Preserves the original Save as Copy functionality without breaking existing save operations.

Testing Instructions

-Go to Components → Banners → Clients in Joomla Administrator. -Select an existing client,. -Click Save as Copy multiple times. -Verify that each new client is created with a unique name: -Confirm that no duplicates are created in the database and all other save functionality works as expected.

Actual result BEFORE applying this Pull Request

-Clicking Save as Copy creates a client with the same name as the original, allowing duplicates.

Expected result AFTER applying this Pull Request

-Clicking Save as Copy automatically generates a unique client name, preventing duplicates while preserving the copy functionality.

Link to documentations

Please select:

• [ ] Documentation link for docs.joomla.org:

• [x] No documentation changes for docs.joomla.org needed

• [ ] Pull Request link for manual.joomla.org:

• [x] No documentation changes for manual.joomla.org needed

[brianteeman]

This is obviously not correct. With this pr you can never Save as copy so why keep the button. Please look at how Save as copy works elsewhere!

[harshkhatri8]

This is obviously not correct. With this pr you can never Save as copy so why keep the button. Please look at how Save as copy works elsewhere!

Thanks for the feedback, @brianteeman. I see your point now — my current PR prevents Save as Copy from working as intended. I’ll update the PR so that:

Save / Save & Close still prevent duplicates.

Save as Copy will create a copy automatically, appending a suffix like it works if a client with the same name exists.

This should fix the original issue while keeping Joomla’s copy functionality intact. I’ll push an updated version soon.

[harshkhatri8]

This is obviously not correct. With this pr you can never Save as copy so why keep the button. Please look at how Save as copy works elsewhere!

i have made all the necessary changes can you please now review it.

[richard67]

@harshkhatri8 When you create a PR you should check how your changes look on GitHub: https://github.com/joomla/joomla-cms/pull/46239/files . Then you would see it has a bunch of code style issues.

[HLeithner]

This pull request has been automatically rebased to 5.4-dev.

[richard67]

@harshkhatri8 I've allowed myself to apply the last 2 necessary code style fixes. You can see the details when expanding the last 2 resolved conversations at the bottom.

[richard67]

@harshkhatri8 In addition to the previous 2 code style fixes, I have applied another 2 to remove the obsolete new language string and use statement. As before you can check the details when expanding the resolved conversations on GitHub.

[exlemor]

@richard67 is this testable now?

[richard67]

@richard67 is this testable now?

@exlemor Yes.

[exlemor]

I have tested this item :white_check_mark: successfully on 435f3ffb3fd0bded181e729ca1564ec6faf8df07

I have tested this successfully! Thanks @harshkhatri8

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46239.}

[ceford]

I think attempting to save a duplicate should return an error and not a duplicate - otherwise items are created by accident.

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46239.}

[chmst]

in the issue #https://github.com/joomla/joomla-cms/issues/45603 I wrote that I expect an error message.

@harshkhatri8 thank you so much for your contribution. But in this case a simple error-message is better. When items are saved as copy, Joomla generates automatically a new title. But this is another use case and if there is a duplicate, it is probably a typo or someone did not see that a client is altready saved.

[brianteeman]

It is perfectly valid to have multiple client with the same name

[ceford]

It is perfectly valid to have multiple client with the same name

In the Banners: Edit form, Banner Details tab you can select a Client from a list of Clients. Would it not be confusing to have clients with the same name?

[harshkhatri8]

Thanks for the suggestions and all the doubts but i have made the changes that are suggested to me in the starting when i start working on the issue. Anything else that I can help with?

[muhme]

@ceford wrote:

I think attempting to save a duplicate should return an error and not a duplicate - otherwise items are created by accident.

@chmst wrote:

in the issue ##45603 I wrote that I expect an error message.

For me, the use case for ‘Copy and Save’ is as follows: I configure one entry with some details, then use ‘Copy and Save’ to duplicate all configurations, and afterwards I modify the new entry. This is a universal UX pattern, and for that, the PR is working as expected for me.

[muhme]

What feels a bit strange to me is that the banner client name is silently changed if I try to have two clients with the same name. For example: I have 'Unicorn Client'. When editing a second entry and changing its name to ‘Unicorn Client’, then pressing Save, it shows the message 'Client saved.' – but it is actually saved as 'Unicorn Client (2)'.

[muhme]

@harshkhatri8 A minor imperfection is that saving a third time adds another ' (2)', resulting in 'Unicorn Client (2) (2)'. In other places, and as I would expect, it would be 'Unicorn Client (3)'. Perhaps it’s best to wait until we’ve decided how to handle the original issue before adjusting this detail.

[harshkhatri8]

@harshkhatri8 A minor imperfection is that saving a third time adds another ' (2)', resulting in 'Unicorn Client (2) (2)'. In other places, and as I would expect, it would be 'Unicorn Client (3)'. Perhaps it’s best to wait until we’ve decided how to handle the original issue before adjusting this detail.

thanks for understanding the problem and explaining in detail it'll help to solve the problem with precision and without creating any misunderstanding.

[muhme]

As discussed today in the maintainers' meeting:

1. 'Save as copy' should add a running counter e.g. ' (2)' if the banner client name already exists.
2. The running counter must continue to be incremented, as elsewhere “ (2)”, “ (3)” … The PR has to be corrected. To use the method like in content article aliases.

@harshkhatri8 Could you please update the implementation, description and test instructions? Thank you in advance.