Trim Package Name of unwanted information

[RhysLees]

This PR filters unwanted information out of the package name.

Resolves #212

[marcusmoore]

Thanks for your quick PR (and all of the PRs you have been submitting lately!).

I think we can simplify this PR by moving the logic to a new accessor on the Package model (something like getDisplayNameAttribute) and potentially using Laravel's string helpers to remove the unwanted strings.

This isn't tested but something like

$toRemove = [
  'nova 4',
  'nova4',
  '(nova 4)',
  'v4',
  'for nova',
  'for v4',
  "()",
  'laravel nova',
];

Str::of($name)->remove($toRemove, false)->trim();

It would be good to have tests around this as well. I'm thinking of a test in Tests\Unit\PackageTest that uses a data provider to test for the following (plus any others you can think of):

Package Name	Is Converted To
CKEditor4	CKEditor4
R64 Fields	R64 Fields
Nova ABC	ABC
Nova 4 ABC	ABC
Nova4 ABC	ABC
ABC for Nova v4	ABC
ABC nova v4	ABC
ABC for v4	ABC
ABC (Nova 4)	ABC
ABC v4	ABC

What do you think?

[RhysLees]

Thanks for your quick PR (and all of the PRs you have been submitting lately!).

I think we can simplify this PR by moving the logic to a new accessor on the Package model (something like getDisplayNameAttribute) and potentially using Laravel's string helpers to remove the unwanted strings.

This isn't tested but something like

$toRemove = [
  'nova 4',
  'nova4',
  '(nova 4)',
  'v4',
  'for nova',
  'for v4',
  "()",
  'laravel nova',
];

Str::of($name)->remove($toRemove, false)->trim();

It would be good to have tests around this as well. I'm thinking of a test in Tests\Unit\PackageTest that uses a data provider to test for the following (plus any others you can think of):

Package Name Is Converted To
CKEditor4 CKEditor4
R64 Fields R64 Fields
Nova ABC ABC
Nova 4 ABC ABC
Nova4 ABC ABC
ABC for Nova v4 ABC
ABC nova v4 ABC
ABC for v4 ABC
ABC (Nova 4) ABC
ABC v4 ABC
What do you think?

Yeah, ill give a test with this approach using Str helper but i still think we should probably use the section where it iterates through the version numbers from 1 to latest major so it will auto-extend the list when a new major version comes out. this will future and past proof the implementation.

[marcusmoore]

That is a good point 👍🏾

[RhysLees]

That is a good point 👍🏾

Just pushed some updates, I noticed an issue caused by the order of the filter list where filters for newer versions were filtering text but not version numbers which caused failures so i swapped the loop around which fixed the issue:
Old Generation

'For Laravel Nova 4',
'For Nova 4',
'N4',
'(4)', // Place at bottom
'For Laravel Nova 3',
'For Nova 3',
'N3',
'(3)', // Place at bottom

New Generation:

'For Laravel Nova 4',
'For Laravel Nova 3',
'For Nova 4',
'For Nova 3',
'N4',
'N3',
'(4)', // Place at bottom
'(3)', // Place at bottom

The only thing I can see being an annoyance is the order of the filter list in config being important for example putting ! at the top would mean Nova ! would never be filtered leaving Nova but i suppose that is what the test is for

[marcusmoore]

Thanks! One more request to add a few more test cases.

Let me know if you want me to take over at any point as well 😄

[RhysLees]

@marcusmoore I made them changes as requested btw.

[marcusmoore]

Thanks @RhysLees. I haven't had a chance to check this out yet but plan on getting back to it later this week.

[marcusmoore]

Thanks @RhysLees 😄