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.

Sign up for GitHub

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

Jump to bottom

support data-suggest- or data-default- with data-set- #2817

Open
robinkar opened this issue May 15, 2023 · 7 comments · May be fixed by #4044
Open

support data-suggest- or data-default- with data-set- #2817

robinkar opened this issue May 15, 2023 · 7 comments · May be fixed by #4044
Assignees
@HazelGrant
Labels
community request component/batch_connect
Milestone
4.1

Comments

@robinkar
Copy link
Contributor

robinkar commented May 15, 2023 •
edited by sync-by-unito bot
Loading

I happened to find a case where the dynamic form JS (BC_DYNAMIC_JS) does not behave as I would expect it to behave in 3.0, and here is an small example case where it happens.
This seems to happen when a field A (project) depends (data-option-for) on field B (reservation), field C (module) depends on field A (project) and sets (data-set) field B (reservation).

In the example config below, setting the project to project_1 with module pytorch makes it impossible to change the reservation, even though only R4 would be an invalid reservation for that particular module and project combination. Meanwhile, the version field, set in the same way as the reservation, is freely selectable.
Changing data-option-for-reservation-R4: false on the project field to some value that does not even exist, e.g. data-option-for-reservation-R5: false, or even just data-option-for-reservation-R4: true still causes the same behaviour.

This behaviour seems to occur in 3.0.1, but not in 2.0.32 where it works properly, i.e. reservation freely selectable, but set to correct default value.

Form config example:

# form.yml.erb
attributes:
  project:
    widget: "select"
    options:
      - [
          "project_1",
          data-option-for-reservation-R4: false,
        ]
      - "project_2"
  module:
    widget: "select"
    options:
      - "tensorflow"
      - [
          "pytorch",
          data-set-reservation: "R3",
          data-set-version: "V1",
          data-option-for-project-project-2: false
        ]
  reservation:
    widget: "select"
    options:
      - "R1"
      - "R2"
      - "R3"
      - "R4"
  version:
    widget: "select"
    options:
      - "V1"
      - "V2"
      - "V3"

form:
  - project
  - module
  - reservation
  - version

┆Issue is synchronized with this Asana task by Unito
@osc-bot osc-bot added this to the Backlog milestone May 15, 2023
@johrstrom
Copy link
Contributor

Thanks, I'll take a look.

@johrstrom johrstrom modified the milestones: Backlog, 3.1, 2.0, 3.0 May 15, 2023
@johrstrom
Copy link
Contributor

Hi, I'm circling back to this now. I think this is the scenario you describe correct?

steps to replicate:

  • select project_1 from project
  • select pytorch from module

expected behavior:
I can change the reservation from R3 to something different

actual behavior:
I cannot change the reservation from R3

my answer
Because you've chosen pytorch, we set reservation based on this data-set directive - data-set-reservation: "R3".

Basically you're hitting this bit. You changed something (the reservation) we're trying to propagate that event to other event handlers in case they have some related relationship. Obviously the choice of pytorch is relevant here.

So it's not just that we're handling change events - it's more like when something does change, we may need to change other elements of the form given these secondary relationships like the choice of pytorch forcing R3.

ondemand/apps/dashboard/app/javascript/packs/batch_connect.js

Lines 715 to 716 in 6259e46

// now that we're done, propogate this change to data-set or data-hide handlers
document.getElementById(elementId).dispatchEvent((new Event('change', { bubbles: true })));

All that said - given pytorch's data-set-reservation directive, it seems to me that this is the correct behaviour. That you would want to keep/force R3 when pytorch is chosen. I know it may be different behavior from 2.0 , but does it make sense to keep/force R3 when pythorch is chosen?

@robinkar
Copy link
Contributor Author

Yes, that does seem to be the correct scenario.

So far we've been using these data-set-* as mostly guiding/helping the user choose the right options, so they still have the possibility of changing them to something else. Due to this we would prefer having it not be forced, so it would work how the version field here (affected by data-set-version) works in both 2.0 and 3.0, although I can see that forcing the values would be equally correct.

@johrstrom
Copy link
Contributor

Due to this we would prefer having it not be forced, so it would work how the version field here (affected by data-set-version) works in both 2.0 and 3.0, although I can see that forcing the values would be equally correct.

Maybe we need a data-suggest- ? That'll set the value once and only once? It's sort of a tough situation because yes you want to set reservation to R3 but only under certain circumstances?

We're mostly using data-set to set hidden values like reservation + account combinations where you choose the account but the reservation is automatically set to something.

Happy to have the conversation about expanding this to suit your needs.

@robinkar
Copy link
Contributor Author

robinkar commented Sep 1, 2023

I feel like the current data-set- option is what my idea of data-suggest- would be, except for this edge case, while the functionality of a forced data-set- attribute can be achieved right now with hidden fields or some logic elsewhere as you mentioned.
One case where we use this is an app where the amount of resources is defined for each module using data-set-. These resources options are hidden, but by checking a checkbox the user can show them and choose to override the defaults for the module. In that case we'd like the amount of resources to be populated with the default values for that module, while still allowing changes. If the definition of "once and only once" includes updating those values once the module changes, even though the user may have changed the resource values, that is what we'd like.

In this particular edge case (issue) it seems like checking if the select option value has changed before applying data-set- attributes could solve the problem. I.e. reservation is changed -> project select options are updated (due to data-option-for-reservation), but results in no changes to value or available options -> module options do not need to be updated (which would trigger the data-set-reservation). Not sure how viable this is with the current code though.

@johrstrom
Copy link
Contributor

OK - I'll turn this ticket into a feature request, though I can't put it in 3.1 yet.

How about some scheme like:

data-set will always set & propogate values.
data-suggest or data-default (don't know the name yet) will only set it once, allowing users to override.

@robinkar
Copy link
Contributor Author

robinkar commented Sep 4, 2023

That sounds good.

@johrstrom johrstrom changed the title Strange behaviour in dynamic form JS in 3.0 support data-suggest- or data-default- with data-set- Sep 6, 2023
@johrstrom johrstrom modified the milestones: 3.0, 4.0 Sep 6, 2023
@johrstrom johrstrom modified the milestones: 4.0, 4.1 Oct 30, 2024
@HazelGrant HazelGrant self-assigned this Dec 19, 2024
@johrstrom johrstrom mentioned this issue Dec 23, 2024
Open
@HazelGrant HazelGrant linked a pull request Jan 2, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@HazelGrant HazelGrant

Labels
community request component/batch_connect
Projects
None yet
Milestone
4.1
Development

Successfully merging a pull request may close this issue.

Data-set-* and Data-suggest-* OSC/ondemand
5 participants
@johrstrom @HazelGrant @robinkar @osc-bot @moffatsadeghi