Regression: not possible to delete source strings in Drupal 8 interface translation

Yeah I think that's an oversight. Not sure if it would be a good idea to integrate in this form but also not sure how to do it otherwise :/

There are also other tables with location info for example to clear out. Also why did you specifically need to clear out strings?

Attached patch adds a checkbox to each row which allows the strings to be deleted and adds a deleteTranslationSource method to the LanguageManagerInterface and LanguageManager class.

It should probably be refactored to include a confirmation page to prevent accidental data destruction.

If we are to do this, we need screenshots, a confirmation form and tests. Also the UI needs to be made clear that not only the translation being seen is removed but all translations and the source string included.

Honestly I am not sure that the added UI shown on all string edit screens will be that valuable. You would barely use this UI to delete things. In fact why do you think its important to delete single strings? Drupal 8 core itself has over 8 thousand strings. Removing one or two would not make your site more performant or anything like that.

Also, if a string is actually used somewhere it will show up as a source again (and if it was part of the project originally, it will get its translation update from localize.drupal.org as well), so practically restore itself soon always at least as a source and as translations too unless you actively disabled translation updates.

While I see this is a feature lost, it would be great to understand what do you want to use this feature for.

@facine: so basically fixing bogus behavior in a module... we should discuss whether the constant UI elements added are worth for that edge case :)

I would be very tempted to Won't fix this one and let this feature emerge in contrib. What do you think, Gábor?

I've been thinking about this some more and I think both @Gábor and @penyaskito have a valid point.

Adding these UI elements is not an improvement visually, given the clean layout of the translation form and this functionality would only serve it's purpose in edge cases.
It is also really easy to implement this functionality in a contrib module. Therefor I would vote in favour of Won't fixing this issue.

Also, I don't think this issue should be major.

Re #6 actually I think this feature is useful to delete a string generated by an old version of a module (mostly custom module, because a contrib module has all strings of all released versions on l.d.o). While it won't help much in performance, it makes it possible to have 100% strings translated (without adding bogus translation), and search won't return a string which is never used.

l.d.o serves version specific translations the same way for contrib and core, there is no difference, so outdated core strings may have the same problem as outdated contrib strings.

I agree with @jcisio, having the ability to delete strings is a useful feature when working with a custom module. If you update the code of the module and change some strings, it's nice to be able to delete the originals to get the 100% translation.

Adding another reason. I just ran into this because I discovered the translation was done wrong on a template and ended up adding a new source string for every term that has been searched on. I didn't see any way in the UI to clean it up so I went through and deleted all the junk from the PO file and imported only to find that doesn't work, either. Would be nice to have a way to bulk delete them all.

Having clean-up of outdated translation is separate issue

Having ability to delete could be useful for customized strings

Any news on this?

@thx jcisio

I tested it with 8.7.7 and it works like a charm.

Thanks handkerchief, it was a reroll of a 4 year old patch. Next time anyone tests could you post screenshots of the new feature please? This patch needs also tests in code.

Here's a screenshot. But I'm the wrong guy for tests (code).

/admin/config/regional/translate

Thanks, patch #25 is working great with 8.9.1.
It would be nice to have a "select all" checkbox like we have in a view using VBO, but for now it is very handy!

Added screenshot from #28 to IS so un-tagging.

Since we are doing this, should we not replace db_delete in deleteTranslationSource() with call to Drupal database service as db_delete() is removed in Drupal 9?

No reason to introduce a method in language manager as there's \Drupal\locale\StringDatabaseStorage::deleteStrings()

Also making delete as bulk operation

A bit of clean-up, still NW for tests

Adds support for multiple delete. As deleteStrings() in StringDatabaseStorage can get multiple values returned by dbStringSelect(), dbDelete() needs to check its condition with the IN operator.
Still NW for tests.

Patch in #36 applied well on Drupal 8.9.7

Adding test cases to validate the following things:
- Presence of delete checkbox
- Deletion process.

Verified and tested patch#39 on the drupal 9.3.x-dev version. Patch applied successfully and looks good to me.thanks @mohit_aghera.Adding screenshot for the reference

@Rinku Jacob 13, thanks for the screenshots. They should be put into the Issue Summary. It helps anyone working on an issue if the Summary is always up to date.

Tagging for an issue summary update for having up to date screenshots in the IS and hopefully someone will add the template. Also tagging novice because the updates to the issue summary for this issue are straightforward.

Hi, @quietone,

Added the Issue summary.

Hi @Rinku Jacob 13
I have applied the #39 patch on the 9.3.x-dev version.
The patch is not applied successfully.

Steps to reproduce:
# Goto: Configuration -> User Interface Translation
# Delete string is not appearing on the right side.

Please refer attached screenshots for the same.
Needs to be re-roll to the patch.

Re-roll for 9.3.x

1. +++ b/core/modules/locale/src/Form/TranslateEditForm.php
   @@ -47,6 +47,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
   +        $this->t('Delete'),
   
   @@ -138,6 +139,11 @@ public function buildForm(array $form, FormStateInterface $form_state) {
   +        $form['strings'][$string->lid]['delete'] = [
   +          '#type' => 'checkbox',
   +          '#title' => $this->t('Delete string'),
   +          '#title_display' => 'invisible',
   

   I think it needs better Label - kinda "Delete translation"

   Also the "Delete string" is disappointing as it will delete translation instead of source string

2. +++ b/core/modules/locale/src/StringDatabaseStorage.php
   @@ -526,7 +526,7 @@ protected function dbStringUpdate($string) {
   -      $query->condition($field, $value);
   +      $query->condition($field, (array) $value, 'IN');
   

   that's changing behavior for key fields so will affect performance

   See #2031261: Make SQLite faster by combining multiple inserts and updates in a single query

Addressed comments mentioned in #47 & added interdiff for reference.

I tested this on Drupal 9.3.x, standard install with a second language.

1. Navigate to admin/config/regional/translate
2. See the delete box on the right hand side
3. Tick the box for 'account settings'
4. Scroll to bottom
5. The button text is 'Save translation'
6. Click 'Save translation'
7. Message "The strings have been saved."
8. The source string and the translation 'account settings' are removed.

I did not expect to see 'save translation' as the button text when I am trying to delete something. There was no confirm screen for the deletion and after the delete operation the message is 'The strings have been saved'. I expect a button that is clearly a 'delete' and a confirm screen and then a confirmation telling me what I just deleted.

#47.1 points out that this deletes the string and the translation, it turns out that is how it behaves in Drupal 7. At least by my testing.

This needs more work on the user experience, tagging for a usability review to get direction.

I moved things around in the IS a bit for readability.

Tested #48 on 9.3.16

Deleting a single item works as expected as experienced in #49. However when trying to delete multiple strings by selecting multiple checkbxoes, it throws following error,

Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[HY093]: Invalid parameter number: number of bound variables does not
                                                match number of tokens: DELETE FROM "locales_target"
                                                WHERE "li

Tested #48 on 9.4.9
Same experience as in #49 and same error as in #51.

Applied some changes to #48 patch:
- introduced separate `Delete translations` button that would handle just deletion. It appears when `delete` checkbox is checked. For now I didn't need confirmation step.
- fixed database error when deleting multiple strings.

Note: not sure if `testLocaleStringDelete()` test would pass.

Tested #53 with php 8.1 and Drupal Core 9.5.4. Works perfectly both with single or multiple selections for delete.
RTBTC +1

Tested #53 with 9.5.9 and it works nicely

Same patch as #53 with a typo fixed.

I re-rolled #57 and created an MR for work going forward. The patch is the current state of the MR.

FYI Patch #57 works with the latest Drupal 10.1.6

But #59 can't be applied, maybe works only with the latest core dev?
Drush error: Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2023-12-02/2503893-59-fix-string-reg...

Patch #59 is for the 11.x-dev branch, because that's the current commit target. It should also work with Drupal 10.2.x, which will be out in a few days.