introduce closure to mutate resource or file id for dom use only

[fschade]

Description

in some cases the resource or item id is not a valid dom selector, to be able to mutate the id this introduces a closure to do so.

Related Issue

• Context menu by deleted shown inside of the rows (CERNBox) web#5927

Motivation and Context

valid dom selectors

How Has This Been Tested?

• unit tests
• yarn link and web

Screenshots (if appropriate):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Tests

Checklist:

• Code changes
• Unit tests added
• Documentation added/updated

Open tasks:

• confirmation if it fixes Context menu by deleted shown inside of the rows (CERNBox) web#5927

[pascalwengerter]

I added an offending ID in the examples (e.g. line 641 id: "forest:19",) to test this, and it visibly breaks the component (by always rendering the dropdown menu) even if the regex itself does the job - string "aa:a/a-a" gets turned into "aaaaa"

[pascalwengerter]

I added an offending ID in the examples (e.g. line 641 id: "forest:19",) to test this, and it visibly breaks the component (by always rendering the dropdown menu) even if the regex itself does the job - string "aa:a/a-a" gets turned into "aaaaa"

Would be great to have this covered in test data somehow, too 😇

[fschade]

I added an offending ID in the examples (e.g. line 641 id: "forest:19",) to test this, and it visibly breaks the component (by always rendering the dropdown menu) even if the regex itself does the job - string "aa:a/a-a" gets turned into "aaaaa"

nice thanks, yes this should fail in line 641, the example overwrites the resourceDomSelector here with an unsafe implementation, if you remove the overwrite it will fallback to here and will work.

the case is covered here for an overwrite and here for the default behavior. Thanks for your deep dive ❤️

[pascalwengerter]

I added an offending ID in the examples (e.g. line 641 id: "forest:19",) to test this, and it visibly breaks the component (by always rendering the dropdown menu) even if the regex itself does the job - string "aa:a/a-a" gets turned into "aaaaa"

nice thanks, yes this should fail in line 641, the example overwrites the resourceDomSelector here with an unsafe implementation, if you remove the overwrite it will fallback to here and will work.

the case is covered here for an overwrite and here for the default behavior. Thanks for your deep dive heart

Okay now I've read the actual implementation 🤦🏽 if CERN confirms this works with their EOS IDs (which it should) I'm happy to approve, merge & release!

[pascalwengerter]

I added an offending ID in the examples (e.g. line 641 id: "forest:19",) to test this, and it visibly breaks the component (by always rendering the dropdown menu) even if the regex itself does the job - string "aa:a/a-a" gets turned into "aaaaa"

nice thanks, yes this should fail in line 641, the example overwrites the resourceDomSelector here with an unsafe implementation, if you remove the overwrite it will fallback to here and will work.
the case is covered here for an overwrite and here for the default behavior. Thanks for your deep dive heart

Okay now I've read the actual implementation 🤦🏽 if CERN confirms this works with their EOS IDs (which it should) I'm happy to approve, merge & release!

Not sure how smart it is to overwrite the sanitization function in an example though, creates knots in the heads of people like me haha

[diocas]

@fschade just tested and it fixes the issue. (the 0-9\-_ that you added after was important :))
Not sure if hashing would be more future proof, but is reasonable to expect that letter and numbers would suffice.

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

23.1% Coverage
0.0% Duplication

[fschade]

@kulmann, @pascalwengerter please have a look, i also touched the naming if ..file.. to ..item.. in this PR.
I know that it's not related to this PR's topic but popped up in the rebase

[pascalwengerter]

Changes LGTM, thanks for the additional cleanup!

[pascalwengerter]

This was an unnecessary breaking change in retrospect (it's the OcFilesTable so file as a prefix is fine, slipped through my code review), let's discuss if we want to rollback & re-release or ship a bugfix release tomorrow