canonical / maas-ui Goto Github PK

Create a table component using v2 vanilla that covers the use cases from the juju-react-components table.

At the moment ActionButton takes an explicit width prop, but it should really just determine that from the width of the text.

It would be nice if the table component had a prop for defining the "empty" message and rendered that instead of "0".

Screenshot

The new maas-ui repo is relying consistently on hardcoded values, with no references to any of the vanilla variables. In some places there are at least //TODO notes to replace with variables.

In others it is just magic numbers.

If these fix vanilla problems, please create an issue on the vanilla repo so I can later address and remove the override.

If they are newly built components, please use the vanilla spacing variables, so we can make things consistent.

The websocket messages are:
{"method":"user.delete","type":0,"params":{"id":2},"request_id":2} and {"type": 1, "request_id": 2, "rtype": 1, "error": "(\"Cannot delete some instances of model 'User' be....

For some reason a DELETE_USERS_ERRORS is not dispatched with that error.

Make the basename optional for dev. There's currently an error if it's not set:

Module../src/index.js
src/index.js:19
  16 | const sagaMiddleware = createSagaMiddleware();
  17 | // This UI will be deployed at /MAAS/settings for now, so route everthing to that.
  18 | export const history = createBrowserHistory({
> 19 |   basename: `${MAAS_config.ui.basename}/settings` // eslint-disable-line no-undef
  20 | });
  21 | const composeEnhancers = composeWithDevTools({});
  22 | const middleware = [sagaMiddleware, routerMiddleware(history)];

Use the history api to go back.

Note: this isn't really a bug, depends on your perspective.

While sync messages correctly update state, and components correctly re-render with state changes from selectors, form fields are not updated with new values.

Steps to Reproduce

1. Open a tab with /MAAS/configuration/general in maas-ui.
2. Open a tab with MAAS/settings/general/ in old maas.
3. Update MAAS name.
4. Confirm that state is updated in maas-ui.
5. Confirm that the component re-renders with the new value from the selector (a console.log should suffice).
6. Confirm that the field does not get updated.

Simple Solution

The problem can be trivially solved with this solution, but I'm not sure we want to do that.

I've confirmed that adding enableReinitialize to <Formik> forms works however.

Unfortunately, I think this has UX issues, as having the form automatically update would be surprising and potentially confusing for users (most web apps are not realtime). Changing data for the input the user is currently editing being the worst case scenario!

Discussion

On one hand, changing form data while a form is being edited is surprising, but equally problematic is having a user submit changes without awareness that the state has changed while they have been editing the form.

Would some visual indication to the user (other than the data changing) that the app has received an update from the server help?

Would it be better to give the user a visual indication that the data has changed and provide a way for the user to manually refresh the form?

Is this a UX problem we could give some thought to @lilyvidenova @cassiocassio?

Github has a nice simple model for this with diffs, when a diff is outdated, there's a just a wee label displayed indicating the current view is out of date. Doing anything more than that (e.g. indicating which fields have changed), would add a lot of additional complexity.

• Create add dhcp snippet form.
• Create redux action to save dhcp snippet.
• Dispatch action on save.

@Caleb-Ellis commented on Thu Aug 15 2019

All tables (in Settings to begin with) should follow the same loading pattern: https://zpl.io/ag6ZPQM

• Create redux action to fetch subnets and snippets. (Methods are: subnet.list and dhcpsnippet.list)
• Create reducers for dhcpsnippet and subnets.
• Create a simple DHCP component, dispatch redux actions to fetch the list of snippets and subnets.

Base selectors, actions and reducers are all in single files. These need to be split to handle the multitude of models we will have.

Display the current page in the window title + fetch and display the maas name in the format [page name] | [maas name] MAAAS e.g. "Settings | mymaas MAAS".

In RBAC we had a separate field in state for filter term. That way we could compose a selector for search that would include the filter term and a subset/mutated list of entities (e.g repositories.getForSettings or something that just renames the defaults). We should consider if we want to use the same pattern here. Regardless of the approach we should fix the original issue of repositories not being filtered by "display name".

Relevant Q in reselect README: https://github.com/reduxjs/reselect#q-how-do-i-create-a-selector-that-takes-an-argument

This thing:

It's a common pattern so should probably be a component with associated styles.

To reproduce

1. Visit /MAAS/users/
2. Search for a user.
3. Note that the pagination count is not updated (there will be n empty pages) - this is particularly confusing if you're already on a page other than 1.

Use takeEvery to fix #172 until we have a better solution.

When deployed in MAAS the URL for settings won't not be /settings so we need to be able to configure the basename to match.

#183 (comment)

If I just enter /MAAS/settings I get a blank page - can we make it redirect to MAAS/settings/configuration/general so the user can get the first available form?

Set the global message types as constants, see: #179 (comment).

Currently we pass nested redux payloads to our websockets layer, e.g.:

repositories.fetch = () => {
  return {
    type: "WEBSOCKET_SEND",
    payload: {
      actionType: "FETCH_REPOSITORIES",
      message: {
        method: "packagerepository.list",
        params: { limit: 50 },
        type: 0
      }
    }
  };
};

This does have the advantage of making the saga code very succinct, but it is a bit unusual and in contravention of flux standard actions. Instead, we should probably dispatch a payload that looks more like:

repositories.fetch = () => (
{
  type: "FETCH_REPOSITORIES",
  meta: {
    limit: 50
  }
})     

That way the redux actions have no knowledge of the websocket implementation, which can be encapsulated in a saga (type, params, method etc).

Traceback:

index.js:1375 Warning: `value` prop on `input` should not be null. Consider using an empty string to clear the component or `undefined` for uncontrolled components.
    in input (at Input.js:33)
    in div (at Field.js:48)
    in div (at Field.js:88)
    in Field (at Input.js:22)
    in Input (at FormikField.js:16)
    in FormikField (at ProxyFormFields.js:37)
    in ProxyFormFields (at ProxyForm.js:93)
    in form (at Form.js:6)
    in Form (at ProxyForm.js:92)
    in Formik (at ProxyForm.js:46)
    in div (at Col.js:45)
    in Col (at ProxyForm.js:43)
    in div (at Row.js:6)
    in Row (at ProxyForm.js:42)
    in ProxyForm (created by Context.Consumer)
    in Route (at Routes.js:34)
    in Switch (at Routes.js:21)
    in Routes (at Settings.js:15)
    in div (at Col.js:45)
    in Col (at Section.js:25)
    in div (at Row.js:6)
    in Row (at Strip.js:41)
    in header (at Strip.js:24)
    in Strip (at Section.js:14)
    in div (at Section.js:10)
    in Section (at Settings.js:14)
    in Settings (created by Context.Consumer)
    in Route (at Routes.js:8)
    in Routes (at Main.js:29)
    in Main (created by ConnectFunction)
    in ConnectFunction (at App.js:26)
    in App (created by ConnectFunction)
    in ConnectFunction (at src/index.js:30)
    in StrictMode (at src/index.js:29)
    in Router (created by ConnectedRouter)
    in ConnectedRouter (created by Context.Consumer)
    in ConnectedRouterWithContext (created by ConnectFunction)
    in ConnectFunction (at src/index.js:28)
    in Provider (at src/index.js:27)

Related, Formik returns a default error message when the fields are submitted and null, which could be friendlier.

Error: httpProxy must be a string type, but the final value was: null. If "null" is intended as an empty value be sure to mark the schema as .nullable()

Currently to update formik with errors from redux state, we disable react-hooks/exhaustive-dep instead of passing formikProps as a dependency, as otherwise useEffect is called in an infinite loop.

  useEffect(() => {
    if (Object.keys(errors).length) {
      const formikErrors = {};
      const invalidValues = {};
      Object.keys(errors).forEach(field => {
        formikErrors[field] = errors[field].join(" ");
        invalidValues[field] = formikProps.values[field];
      });
      formikProps.setStatus({ serverErrors: formikErrors, invalidValues });
    }
  }, [errors]); // eslint-disable-line react-hooks/exhaustive-deps

When a user is added/updated there should be a message on the user list.

Make the user delete button actually delete users over the API.

Currently WEBSOCKET_ERRORs are set in state, but not displayed to the user.

This is probably an issue with only some of the config/general data being hydrated when an image starts downloading, and the rest of it is hydrated only when the image has finished downloading.

In the websocket saga the messages are never taken when dispatched by components with useSelector returning a new state. See this minimal reproduction:

https://gist.github.com/huwshimi/0f95fa8a77874851370a4a49a2aca3ae

A working version can be found here: https://codesandbox.io/s/affectionate-mahavira-diihd

It would be ideal to construct RPC methods from meta.model and meta.method in redux actions, however this will require us to unify the top level models in redux state with the API models.

e.g. repositories -> packagerepository

Create default configurations for prod, dev and the demo service. Maybe using environment variables to load the correct config.

If maas server has csrf authentication enabled, the client will sit in a "loading" state indefinitely without an error.

The client should be raising a PROTOCOL_ERROR with the error "Invalid CSRF token", but this is not surfaced to the client.

Convert the p-strip and elements to a reusable component. See: https://docs.vanillaframework.io/patterns/strip/

These are required by the dhcp snippet list.

Relevant PR comments: #133 (review)

At the moment, general data is only fetchable and in many cases its in some way derived from config state. We need a way to re-fetch general state whenever config is synced (UPDATE_CONFIG_NOTIFY).

Currently we use the hook useFetchOnce to ensure data is only fetched once per model. This should

To reduce duplication in the table views the pagination controls could be moved into the MainTable component.

The FormikField component should generate unique form field ids to prevent collisions.

Use the cookie to get the CSRF token when deployed alongside MAAS. A CSRF token should also be able to be provided in dev when the cookie is not available.

At the moment the button doesn't actually reflect state. Instead it assumes moving from saving: true to saving: false means the action was successful, which is pretty naive.
There should be some value in state that is set to true when a form submission is successful (or when config is synced) that the ActionButton receives.
The ActionButon should still have some internal state that determines how long to show the spinner and success icons, otherwise the spinner duration could be almost instant, and the success duration could be forever.

• Create a confirmation to delete a dhcp snippet (as an expanding row).
• Create new redux action to delete a dhcp snippet.
• Dispatch action on clicking delete, and close confirmation if successful.

Create a selector for dhcp snippets and display the data in a table with edit/delete buttons (non-functional).

Create edit form for DHCP snippet.
Create redux action to save dhcp snippet.
Dispatch action on save.

When changes happen in MAAS e.g. from the CLE, then presumably there are websocket messages sent to the client which are not handled by the normal get/request implementation we have now.

This might require having a mapping of request types to action type e.g. users.list to UPDATE_USERS, but we need to investigate what the messages contain.

Config is never fetched if app loaded from /users or non config route, and config components relying on state never load.

Steps to reproduce

1. Load app from /MAAS/users
2. Visit /MAAS/configuration/general

At lower window widths, table cell contents overflow.

e.g. email in the users table

Check that the authenticated user is a superuser and if not then disable all settings views.