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

Request column names from server when loading table #6375

Merged
merged 32 commits into from Feb 1, 2024

Conversation

SchrodingersGat
Copy link
Member

This PR adds facility for tables to fetch the translated column names from the server, using the existing metadata endpoint. This allows us to reduce duplication of translated strings between the server and client.

As each table is constructed from data provided by a defined endpoint, and (most) API fields have localized label attributes, we can use those to render the localized table columns.

Note that the table column titles can be manually overridden if required.

If this is deemed a good idea, following improvements can be made:

  • Cache the column names based on the table key
  • Update existing tables to not specify column names where not required.

- As these are translated by the server, we can make use of them on the client side!
- Reduces duplication of translated titles
@SchrodingersGat SchrodingersGat added enhancement This is an suggested enhancement or new feature user interface User interface Platform UI Related to the React based User Interface labels Jan 31, 2024
@SchrodingersGat SchrodingersGat added this to the 0.14.0 milestone Jan 31, 2024
Copy link

netlify bot commented Jan 31, 2024

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

Name Link
🔨 Latest commit e6f9b69
🔍 Latest deploy log https://app.netlify.com/sites/inventree-web-pui-preview/deploys/65bc1b9466770b0008915333
😎 Deploy Preview https://deploy-preview-6375--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 (no change 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 SchrodingersGat marked this pull request as draft January 31, 2024 13:42
@matmair
Copy link
Member

matmair commented Jan 31, 2024

Looks/Sounds intresting and relativley simple to implement. Maybe caching all table column names similar to states would be a good idea?

@SchrodingersGat SchrodingersGat marked this pull request as ready for review February 1, 2024 06:32
@wolflu05
Copy link
Contributor

wolflu05 commented Feb 1, 2024

Nice idea, LGTM now after caching is implemented.

src/frontend/src/functions/string.tsx Outdated Show resolved Hide resolved
src/frontend/src/tables/ColumnRenderers.tsx Show resolved Hide resolved
src/frontend/src/tables/InvenTreeTable.tsx Outdated Show resolved Hide resolved
src/frontend/src/tables/InvenTreeTable.tsx Outdated Show resolved Hide resolved
src/frontend/src/tables/sales/SalesOrderTable.tsx Outdated Show resolved Hide resolved
SchrodingersGat and others added 4 commits February 1, 2024 08:17
- We will rely on the server-side translations!
Co-authored-by: Matthias Mair <code@mjmair.com>
@SchrodingersGat SchrodingersGat changed the title [WIP] Request column names from server when loading table Request column names from server when loading table Feb 1, 2024
@SchrodingersGat SchrodingersGat merged commit ec2a66e into inventree:master Feb 1, 2024
24 checks passed
@SchrodingersGat SchrodingersGat deleted the table-column-names branch February 1, 2024 23:11
@wolflu05
Copy link
Contributor

wolflu05 commented Feb 2, 2024

This has now broken my machine integration tables. E.g. this API endpoint is used in a table:

https://github.com/wolflu05/InvenTree/blob/34d730febf68dc5e0851837955797e1866042f19/InvenTree/machine/serializers.py#L110
https://github.com/wolflu05/InvenTree/blob/34d730febf68dc5e0851837955797e1866042f19/InvenTree/machine/api.py#L157-L172

And PUI always shows a notification on load:

image

After investigation, it seems like that this is coming from:

if (!actions) {
notifications.show({
title: t`Form Error`,
message: t`Response did not contain action data`,
color: 'red'
});
return null;
}

Because when using an APIView for non model api endpoints this results in no actions added to the OPTIONS response.

Same things applies for the Plugin page in the Admin center.

image

How should we fix this?

@SchrodingersGat
Copy link
Member Author

@wolflu05 you found that so quickly! I had just been looking into this. I think that we should

a) Remove the "form error - response did not contain action data" - it is a very "nerdy" error which I had in there for debugging. Probably not very useful
b) Where is the "permission denied" warning coming from?

@wolflu05
Copy link
Contributor

wolflu05 commented Feb 2, 2024

Yes, was trying to resolve the merge conflicts on the machines branch and refactor the tables, ...

Yes, I'm on board with just removing this notification. The enduser doest know what that means. Do you create an PR for that?

Regarding the permissions error, I guess that is coming from here:

if (!(method in actions)) {
// Missing method - this means user does not have appropriate permission
permissionDenied();
return null;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is an suggested enhancement or new feature Platform UI Related to the React based User Interface user interface User interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants