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

[PUI] Enable passing of string urls to apiUrl #6012

Merged
merged 1 commit into from
Dec 2, 2023

Conversation

matmair
Copy link
Contributor

@matmair matmair commented Nov 30, 2023

This PR enables passing strings (preferably URLs) to API forms so they can be used with custom URLs / in plugins.

@matmair matmair added plugin Plugin ecosystem Platform UI Related to the React based User Interface labels Nov 30, 2023
@matmair matmair self-assigned this Nov 30, 2023
Copy link

netlify bot commented Nov 30, 2023

Deploy Preview for inventree-web-pui-preview ready!

Name Link
🔨 Latest commit f9c6766
🔍 Latest deploy log https://app.netlify.com/sites/inventree-web-pui-preview/deploys/6568318a19ca3c0007abf43c
😎 Deploy Preview https://deploy-preview-6012--inventree-web-pui-preview.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 100 (🟢 up 1 from production)
Accessibility: 86 (no change from production)
Best Practices: 92 (no change from production)
SEO: 70 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@SchrodingersGat
Copy link
Member

This will weaken the existing type checking checks which have already been useful in detection of mis-typed URLs. However I am not sure if there is a better approach here

@matmair
Copy link
Contributor Author

matmair commented Dec 1, 2023

I am very open to better implementations, this is just the most minimal API change I could come up with

@wolflu05
Copy link
Contributor

wolflu05 commented Dec 1, 2023

In my opinion the idea behind this function is good, but the issue with it is, that if adding an api path, I basically have to go to two different files and add something on two places. That slows down the quick development. As the apiEndpoint function is just a huge switch case prefabricated building, why not just use an object with key: url and type the function accordingly. And then to allow strings, we add a second function that allows strings which gets called by the function which only allows ApiPaths. Because we cannot union string with choices. E.g. something like this:

export const API_PATHS = {
  "api-plugin-settings": "plugins/:plugin/settings/",
  // ...
}
export type API_PATH_TYPE = keyof typeof API_PATHS;

export function apiGenericUrl(path: string; ...) {
  // do  like before, but without the apiEndpoint call
} 

export function apiUrl(path: API_PATH_TYPE, ...args) {
  return apiGenericUrl(API_PATHS[path], ...args)
}

That way we would

  1. only need to change something at one place
  2. only need to import the apiUrl function
  3. have the possibility to use raw strings without mixing them

@SchrodingersGat SchrodingersGat merged commit bbf4d2f into inventree:master Dec 2, 2023
27 checks passed
@SchrodingersGat
Copy link
Member

@wolflu05 I'd suggest making that as a separate issue :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Platform UI Related to the React based User Interface plugin Plugin ecosystem
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants