Altinn / app-frontend-react

Altinn application React frontend
BSD 3-Clause "New" or "Revised" License
16 stars 24 forks source link

Form element: Generic button #133

Closed Febakke closed 5 months ago

Febakke commented 5 years ago

Description

Look into if service developers needs a generic button that can be used for different functions with some coding. If there is a need for a button with a specific function decide if the button type should be standard components. (example: cancel or run calculations)

This was originally included in issue Altinn/altinn-studio#1025, but was seen as out of scope after discussion with developers.

Technical considerations

Input (beyond tasks) on how the user story should be solved can be put here.

Acceptance criterea

Out of scope

Specification tasks

lvbachmann commented 5 years ago

@Febakke What is the difference between this and Altinn/altinn-studio#1025?

Febakke commented 5 years ago

@Febakke What is the difference between this and Altinn/altinn-studio#1025?

This issue handles how to implement a generic button, a button service developers can use to trigger something. the functionality is potentially more complex than a submit button that only has one function.

olemartinorg commented 8 months ago

Some thoughts about this has been spinning in this head for about a week now. @tjololo created the above linked task for the backend API, and we had a discussion where we concluded (or, well, this is a recollection of what I remember us concluding) that:

  1. The generic button should trigger a request to the backend when pressed.
  2. The backend replies back with a structured JSON object that describes the action the frontend should take. (One that we can extend in the future with more possible actions).

After thinking about it, I want to also suggest some more details:

  1. The backend replies back with an array of actions for the frontend to execute. Each one of these should be implemented in frontend as async methods, where we wait for one to complete before we move on to the next action to perform. Most implementations might very well just send back one action, but that's fine.
  2. We already have a secure flag for options (where we call a different endpoint that makes the backend code execute in an instance-aware context). We could use something similar here.
  3. We also have mapping for options, where we can pull data from the data model into query parameters, but I suggest following my proposed solution in #1181 for a more flexible solution using expressions for query parameters instead.
  4. We could support (possibly at a later time) statically-defined actions, or statically defined actions using expressions, thus skipping a call to the backend. In principle, a generic button that is statically configured to trigger [{ action: 'validateAllPages' }, { action: 'navigateToNextProcessStep' }] (or something like that) would behave the same as our current Button (aka the submit button). Assuming, of course, that validateAllPages is an action that rejects if not all pages validate correctly (and prevents attempting to move to the next process step).
  5. In the future, this functionality (of defined actions, possibly with parameters) should supersede our current triggers system. It is ambiguous at which point a trigger on current components actually triggers (!), as a trigger on an Input component triggers when data in the component is saved (or rather, after it is saved), as it's only used for validation. For a repeating Group component however, triggers run when pressing the button to save and close the row. In the future it would be more flexible if one could define when the action triggers, and it could allow for far more interesting action types. I.e.:
    {
    "type": "Group",
    "maxCount": 50,
    "triggers": {
    "afterPressingEdit": [
      [
        "if",
        ["equals", ["dataModel", "@remotelyProcessed"], true],
        ["list",
          { "action": "alert", "title": "text.resource.sorryDataHasBeenProcessedAlready" },
          { "action": "cancel" }
        ],
        "else",
        { "action": "continue" }
      ]
    ]
    }
    }

Also, some thoughts on which actions we could support:

In this model, we might also need some kind of error handling, i.e. catching and handling of errors.

More thoughts:

tjololo commented 8 months ago

If I have understood your ideas correctly the trigger part of the GenericButton type are actions that frontend should know how to handle by itself and would not need to be sendt to the backend? If so then we should remember that the ActionButton uses the field action to tell the backend what to do when it's pressed. To not create confusion about what action refers to.

{
  "id": "Button-Lj0cYD",
  "type": "ActionButton",
  "action": "sign",
  "textResourceBindings": {
      "title": "Button-Lj0cYD.title"
  },
  "buttonStyle": "primary"
}

I imagined we could reuse some parts of the config for ActionButton for the new generic button

Reusing the secure pattern from options seems like a good idea, and should only differ on what endpoint the POST should be performed against.

I havn't worked out all the kinks but thought I should share my first thoughts about how the api could look like for generic buttons. Request body:

{
  "action": "populate",
  "metdadata": {
    "key": "value",
    "what": "ever"
  }
}

The metadata map is not required and would make it possible for the app developers to enrich the request with data. Once we get more experience with how the generic buttons are used we could choose to extend the model with more specialised fields.

Once the API receives the request our code authorises that the user is allowed to perform the action specified and then routes the request to the app developers custom code. They do their work and return a result to our code that translates the result to a respons to frontend

{
  "fieldsChanged": [
    "data.scheme.username",
    "data.scheme.email"
  ],
  "frontendAction": [
    "nextPage"
  ]
}

These are just my first thoughts after reading through your comment, feel free to come with suggestions and feedback🙂

FTLems commented 8 months ago

We defintly need a generic button for limit and control when the IPersonLookup.GetPerson is triggered as there are only 5 attempts before there is a timeout on 1 hour.

Please include org/krt tag here :)

olemartinorg commented 8 months ago

@tjololo Looks good! I agree 💯 with re-using the action-part from ActionButton, so that we can verify permissions as well.

And yes, I'm thinking that in some cases the button can have static configuration as well, not requiring a request to the backend in order to complete. That is, for simple cases like 'navigate to the next page' or 'go to this specific page' once pressed. That would not cause any permissions checks to run, and the result would always be what was configured statically.

And regarding your other suggestions: Metadata: Yup, that could work. Although the way mapping currently works for options/data lists, etc, is that we add query parameters - it could work just as well by adding a metadata object.

Response to frontend: Your suggestion is an object, not an array. Since keys in objects are not guaranteed to be in order in JSON, it's also not possible to guarantee execution order when returning multiple actions. So, say you return 'validate the page' and 'move to the next page' as a result, if we just sent back { "validatePage": true, "moveToNextPage": true } we would have no way of knowing if the intended result was to validate the page before navigating or after navigating. In other words, I think your example has to be rewritten to an array:

[
  {
    "action": "fieldsChanged",
    "fieldsChanged": [
      "data.scheme.username",
      "data.scheme.email"
    ],
  },
  {
    "action": "nextPage"
  }
]

Things to note:

  1. Everything should be an action. Saying some fields have changed on the backend triggers something to happen on the frontend, but the order of these things should be customizable. Although that break apart if you first trigger an action to go to the next process step and then say some fields have changed (in the next data model). I guess backend could force that action to always be first.
  2. Many actions can have metadata attached to them, such as gotoPage. Having a simple list of strings as actions would make that impossible, so they should be objects.

EDIT: We should also consider scrapping 'fieldsChanged' and opt for a full datamodel print-out, as per https://github.com/Altinn/app-lib-dotnet/issues/316

tjololo commented 8 months ago

I didn't think of change fields as an action returned to frontend, but as some additional (optional) information in the object. The actions are a list so they can be ordered. Building the response in backend I think would feel more natural. If we switch to returning whole datamodel is that also an action? would we need to post the whole datamodel multiple times if multiple actions neeeds the datamodel?

olemartinorg commented 8 months ago

Hmm, you're probably right, maybe it shouldn't be an action. The thinking behind this was that the frontend code could just do something like this:

for (action in actions) {
  await perform(action);
}

However, it's probably more correct to do a:

if (response.changedFields) {
  await injectIntoDataModel(response.changedFields);
}
for (action in actions) {
  await perform(action);
}
tjololo commented 8 months ago

We should also consider adding the possibility to return validation issues in the response. One usecase for the button is looking up a person based on SSN and lastname, if no person is found for this combination we should make it possible to return a validation issue telling the user that ssn+lastname yielded no result

bjosttveit commented 7 months ago

Looking over your list of proposed actions to support, maybe we could also support setting focus to any component since we already support this through the summary "edit" button and errorReport?

olemartinorg commented 7 months ago

@bjosttveit! Yess! 👏 Agree. That's a very useful action, especially together with no-backend-action-generic-buttons (😱). It might be useful to have a button to skip to an entirely different component, possibly on another page. In that case we should also support scrolling to that component (not just focusing it), for presentation components (Header, etc).