-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Dynamic warranty link for manufacturers' support urls #12906
Conversation
This pull request has been linked to Shortcut Story #20622: Dynamic manufacturer link. |
PR Summary
|
Additionally, a byproduct of this change will open up the support url field to be used for more diverse links/clickthroughs rather than strict web urls |
$this->merge([ | ||
'support_url' => $this->input('support_url'), | ||
]); | ||
dd($this->all()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably don't want to leave a dd()
in there
app/Models/Manufacturer.php
Outdated
'support_email' => 'email|nullable', | ||
'support_url' => ['regex:/.+:\/\/.+/','nullable'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I mentioned on the dev call, we might be able to simplify this a bit, using the starts_with
validation.
'support_url' => 'nullable|starts_with:http://,https://',afp://,facetime://,file://,irc://'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks really good - just a few changes!
app/Models/Manufacturer.php
Outdated
'support_email' => 'email|nullable', | ||
'support_url' => ['starts_with:http://,https://,afp://,facetime://,file://,irc://','nullable'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to use brackets here? I don't think we do - just
'support_url' => 'nullable|starts_with:http://,https://,afp://,facetime://,file://,irc://',
should do it, no? I'd prefer that for consistency if it works.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all set
@@ -3,6 +3,7 @@ | |||
namespace App\Http\Controllers; | |||
|
|||
use App\Http\Requests\ImageUploadRequest; | |||
use App\Http\Requests\SaveManufacturerRequest; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all set
A way to dynamically produce a manufacturur's warranty link has been added.
Instead of building out code per individual manufacturer which is unscalable, this method will allow any manufacturer to have a warranty link be customized for use.
SupportURLs can now be written with
{LOCALE}
and{SERIAL}
tags in them. Using these tags, the system will automatically replace{LOCALE}
with the localization variable selected in settings.{SERIAL}
will be replaced with the serial number of the asset being viewed.For example:
-strict URL validation has been removed.
-urls must have
://
in them to validate.-Other special characters are now allowed as long as
://
is in the link.-No existing urls or presaved links should be affected, but will need to have
://
if the manufacturer is edited in the future.-urls are free to not use the tags
-the individual icons for manufacturers have been changed to a default "outside link" icon next to the warranty months
How Has This Been Tested?
Tested in local. on existing manufacturers and with newly created manufacturers.
Checklist: