Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove "N4", "v4", "Nova 4", "4", "(4)" From name #212

Closed
RhysLees opened this issue May 15, 2022 · 7 comments · Fixed by #220
Closed

Remove "N4", "v4", "Nova 4", "4", "(4)" From name #212

RhysLees opened this issue May 15, 2022 · 7 comments · Fixed by #220

Comments

@RhysLees
Copy link
Contributor

I think it would be best to remove any mention of version from Package names and instead encourage people to require nova ^4.0 in their composer.json to show support

@marcusmoore
Copy link
Contributor

I agree and you beat me to creating an issue for it!

@RhysLees
Copy link
Contributor Author

I agree and you beat me to creating an issue for it!

If I get time this week I might make a PR

@RhysLees
Copy link
Contributor Author

@marcusmoore If you could point me to the file where its best to trim the names ill submit a pr.

@marcusmoore
Copy link
Contributor

It looks like the replacement is happening in the templates.

I would like to move that code to a presenter method of some sort so we can have a place to put future replacements and write tests around it.

It would be great if you're up for working on it but no pressure (of course 😄 )

@RhysLees
Copy link
Contributor Author

It looks like the replacement is happening in the templates.

I would like to move that code to a presenter method of some sort so we can have a place to put future replacements and write tests around it.

It would be great if you're up for working on it but no pressure (of course 😄 )

I'll see what I can do!

Just a thought, what if we just make a custom validation rule for name field, that way we don't even save the 'unnecessary data' to db and the user will be displayed an error, which we can tell them to use composer versions to specify support. That would also keep it in one place so we don't duplicate code.

@marcusmoore
Copy link
Contributor

Thanks and no rush @RhysLees 😄

I don't know if I want to reject a package that includes the version in the title but I think we should be more clear about how we determine v4 support. I opened an issue to track that here: #218

@RhysLees
Copy link
Contributor Author

RhysLees commented May 24, 2022

Thanks and no rush @RhysLees 😄

I don't know if I want to reject a package that includes the version in the title but I think we should be more clear about how we determine v4 support. I opened an issue to track that here: #218

Ah ok, i was going to recommend the following as a rule. would allow is to return feedback to the user when they create the form. This can also be used with a tiny modification to take it out of the array:

    /**
     * Determine if the validation rule passes.
     *
     * @param  string  $attribute
     * @param  mixed  $value
     * @return bool
     */
    public function passes($attribute, $value)
    {
        /**
         * Any item in the haystack that doesn't contain a string needs to be placed at the bottom
         * otherwise it will leave the string before the version number.
         */
        $invalidSubstrings = [
            'N!',
            'V!',
            'Nova !',
            'Nova!',
            '(!)', // Place at bottom
            '!', // Place at bottom
        ];

        // The haystack used to check if the string contains any of the invalid substrings.
        $versionHaystack = [];

        // Create the version haystack for each version of Laravel Nova.
        $v = 1;
        while ($v <= config('novapackages.nova.latest_major_version')) {
            foreach ($invalidSubstrings as $subject) {
                // Replace ! with the version number.
                $versionHaystack[] = str_replace('!', $v, $subject);
            }

            $v++;
        }

        // Filter the value to only contain strings that contain the version number case insensitive.
        $filteredVersions = str_ireplace($versionHaystack, '', $value);

        // Filter extra spaces created by the removing elements from the version haystack.
        $filtered = preg_replace('!\s+!', ' ', $filteredVersions);

        return $filtered === $value;
    }

    /**
     * Get the validation error message.
     *
     * @return string
     */
    public function message()
    {
        return 'The :attribute must not reference a Laravel Nova version. Require the version in your "composer.json".';
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants