Change Links to Link(s) to match the rest of the messages

[n8-dev]

Description

Tweaked messaging to handle the situation when context is both Plural and Singular.
Much like how it already is here.

    "LinkField.FAILED_TO_LOAD_LINKS": "Failed to load link(s)",

i.e.

- "LinkField.SAVE_RECORD_FIRST": "Cannot add links until the record has been saved",
+ "LinkField.SAVE_RECORD_FIRST": "Cannot add link(s) until the record has been saved",

Manual testing steps

Update text should show when loading an item that has a relation to 1/many linkfields that isn't saved and you wish to add a link to it.

Issues

• Plural and Singular CMS Message update #359

Pull request checklist

• The target branch is correct
  • See picking the right version
• All commits are relevant to the purpose of the PR (e.g. no debug statements, unrelated refactoring, or arbitrary linting)
  • Small amounts of additional linting are usually okay, but if it makes it hard to concentrate on the relevant changes, ask for the unrelated changes to be reverted, and submitted as a separate PR.
• The commit messages follow our commit message guidelines
• The PR follows our contribution guidelines
• Code changes follow our coding conventions
• This change is covered with tests (or tests aren't necessary for this change)
• Any relevant User Help/Developer documentation is updated; for impactful changes, information is added to the changelog for the intended release
• CI is green

[n8-dev]

Working on updating the dist file

[GuySartorelli]

Please remove all of the changes to localised files for languages other than English - those are managed in transifex. Feel free to update the references in transifex if you like, they'll get pulled down whenever our next localisation run is (I think those are every 3 months if I'm remembering correctly)

[GuySartorelli]

Actually, looking at this - we should probably have two separate messages. The javascript component knows whether this is a multi-link field or a single link field, so we should always use the existing "links" message for a multi link field and use a new "link" message for single link fields.

[n8-dev]

Linkfield isn't in https://explore.transifex.com/silverstripe/ ?

[GuySartorelli]

Weird. It's definitely in the app: https://app.transifex.com/silverstripe/silverstripe-linkfield/dashboard/
I'll have to look into how that explore page gets its info

[n8-dev]

Cool, can see that, is there anything specific to do with the new option added?

[GuySartorelli]

Nah, that'll get collected and added to transifex when we do our next localisation round.

[n8-dev]

Think this is ready.

Unsure about a few things...

• ENH vs TRN
• I think this is the right target based on change level...
• Is CI green?

[GuySartorelli]

• ENH is correct here since you're enhancing the functionality to include a second separate message, you're not only affecting the localisation strings and keys.
  • Usually the colon shouldn't be included, i.e. it should be ENH My commit message here. not ENH: My commit message here.. It doesn't matter much really, but since you need to make changes anyway (see below about reverted change) you may as well fix that at the same time.
• Target branch is correct
• CI doesn't look like it has run for some reason (you can see in the "checks" tab at the top, it's got 0 items) - GitHub does this sometimes. Another force push usually gets it to right itself.

You've reverted #360 (comment) and #360 (comment) in your commit squashing efforts, though.

[n8-dev]

Dang!

Onwards!

[n8-dev]

Hmm, workflow awaiting approval?

https://github.com/silverstripe/silverstripe-linkfield/actions/runs/12624941936

This because it's a fork PR and something isn't connected quite right?

[n8-dev]

Now its running idk 🤷

[GuySartorelli]

Oh yeah, I forgot the new merge experience doesn't actually tell me when I need to approve CI runs, nor gives me the option to do so. Had to swap back to the old one. 😓

[n8-dev]

Hmm, only 1 version is failing on the CI. Could it be a isolated thing?

[GuySartorelli]

Looks like CI doesn't like your dist build. Did you remember to rebuild it after updating #360 (comment) and #360 (comment)?

only 1 version is failing on the CI

There is only one version of the JS CI job

[n8-dev]

Will re-rebuild 🤷

only 1 version is failing on the CI

There is only one version of the JS CI job

I mean only mysql57 failed 🤔

[GuySartorelli]

I mean only mysql57 failed

Right, but it's mysql57 js which is the only js one.
The mysql57 shouldn't really be there and is just a result of the way we create and name the job matrix for CI runs.

We don't use the database for JS tests/linting, so there's no need to create another JS run with a different database, unlike unit and end-to-end tests.

[n8-dev]

Woohoo that passed.

Can we merge and maybe a cheeky release @GuySartorelli ? 🎉

[GuySartorelli]

Once I have tested it and approved it, I will merge it.
Once it has been merged, GitHub Actions will automagically tag it. Sorry forgot what branch it's targetting.
It will be included as part of the April minor release.

[GuySartorelli]

LGTM, works well locally.
I don't understand why that client got confused, given "Cannot add links" includes the single link that field would support... but now there will be less cause for confusion.

[n8-dev]

Maybe confused is not the right word.
They always want to improve CMS details for their CMS authors.

Single links were saying links so reported as Bug. 😄