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

Added #14426: Makes all Manufacturer links dynamic, not just warranty lookup #14470

Closed
wants to merge 35 commits into from
Closed

Conversation

DrekiDegga
Copy link

@DrekiDegga DrekiDegga commented Mar 21, 2024

Description

This makes all manufacturer links dynamic just like warranty lookup.

Added #14426

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

I've tested working on latest from my test environment.

Test Configuration:

  • PHP version: 7.4.33
  • MySQL version 10.11.4-MariaDB-1~deb12u1 Debian 12
  • Webserver version apache2 2.4.56-1~deb11u2
  • OS version Debian 12

Checklist:

snipe and others added 30 commits March 6, 2024 20:01
Signed-off-by: snipe <snipe@snipe.net>
Signed-off-by: snipe <snipe@snipe.net>
Signed-off-by: snipe <snipe@snipe.net>
…14ca3ad2fae7b

[Snyk] Upgrade webpack from 5.90.1 to 5.90.2
Signed-off-by: snipe <snipe@snipe.net>

# Conflicts:
#	package-lock.json
Signed-off-by: snipe <snipe@snipe.net>

# Conflicts:
#	config/version.php
Signed-off-by: snipe <snipe@snipe.net>
Signed-off-by: snipe <snipe@snipe.net>

# Conflicts:
#	public/css/build/app.css
#	public/css/build/overrides.css
#	public/css/dist/all.css
#	public/mix-manifest.json
Signed-off-by: snipe <snipe@snipe.net>

# Conflicts:
#	public/css/dist/all.css
#	public/css/dist/bootstrap-table.css
#	public/js/dist/bootstrap-table.js
#	public/mix-manifest.json
@DrekiDegga DrekiDegga requested a review from snipe as a code owner March 21, 2024 03:55
Copy link

welcome bot commented Mar 21, 2024

💖 Thanks for this pull request! 💖

We use semantic commit messages to streamline the release process and easily generate changelogs between versions. Before your pull request can be merged, you should update your pull request title to start with a semantic prefix if it doesn't have one already.

Examples of commit messages with semantic prefixes:

  • Fixed #<issue number>: don't overwrite prevent_default if default wasn't prevented
  • Added #<issue number>: add checkout functionality to assets
  • Improved Asset Checkout: use new notification method for checkout

Things that will help get your PR across the finish line:

  • Document any user-facing changes you've made.
  • Include tests when adding/changing behavior.
  • Include screenshots and animated GIFs whenever possible.

We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.

Copy link

what-the-diff bot commented Mar 21, 2024

PR Summary

  • Enhancement of the 'Manufacturer' Model
    A new rule was added to the 'Manufacturer' model to accommodate URLs that start with various protocols like http, https, afp, facetime, file, and irc. This change provides more flexibility for the 'url' field.

  • Introduction of New Methods in 'AssetPresenter' Class
    Two methods - dynamicSupportUrl() and dynamicUrl() - were introduced. They help in dynamically generating appropriate URLs for the support and manufacturer fields.

  • Improvement on the 'view.blade.php' File
    The icons and links for both the manufacturer and support URLs have been changed. Rather than having static URLs, the newly introduced dynamic URL generation methods are being used. Hence, the appearance and functionality of the mentioned links are updated, leading to a more interactive user experience.

Copy link
Owner

@snipe snipe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's some duplication we can clean up in this.

app/Presenters/AssetPresenter.php Outdated Show resolved Hide resolved
@DrekiDegga
Copy link
Author

@snipe Sorry about that. I believe I've made the requested changes by merging the Dynamic URL Functions in AssetPresenter.php.

@DrekiDegga DrekiDegga requested a review from snipe March 22, 2024 00:57
DrekiDegga and others added 2 commits March 21, 2024 21:09
@snipe
Copy link
Owner

snipe commented Mar 26, 2024

This looks a lot better, thank you. But c an you retarget to develop, per the developer docs?

@DrekiDegga DrekiDegga changed the base branch from master to develop March 26, 2024 13:05
@DrekiDegga DrekiDegga marked this pull request as draft March 26, 2024 13:06
@DrekiDegga DrekiDegga closed this Mar 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants