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

Review front end models #105

Closed
andrii-i opened this issue Oct 7, 2022 · 3 comments · Fixed by #207
Closed

Review front end models #105

andrii-i opened this issue Oct 7, 2022 · 3 comments · Fixed by #207
Labels
enhancement New feature or request
Milestone

Comments

@andrii-i
Copy link
Collaborator

andrii-i commented Oct 7, 2022

What's confusing is that model.jobsView = 'ListJobs' will render either the job list or the job definition list, dispatching based on model.listJobsModel.listJobsView.

@andrii-i For now, you'll want to somehow set the listJobsView to 'JobDefinitionDetail' immediately after line 29 above.

@jweill-aws We need to have a more thorough discussion on the purpose of these models. To my recollection, the whole point of models as a global state store was to allow persistence (serialization to JSON in local storage) and rehydration upon page reload. However, the list jobs view really doesn't have any state that needs to be persisted. Whether the jobs list or the job definitions list is being rendered should be stored in model.jobsView rather than inside model.listJobsModel.listJobsView, which means model.listJobsModel can be deleted entirely.

Another refactoring suggestion, rename JobsModel => SchedulerModel and model.jobsView => model.view.

Originally posted by @dlqqq in #92 (comment)

@ellisonbg
Copy link
Contributor

@jweill-aws not sure I am following. I think that when the user has the list view open, if they reload the page, the list view should show exactly the same thing. For now, that is showing jobs or job definitions in the tab. As such I think that information should also be in the global model and serialized. In the future, we will also want to serialize the column sorting information, and the search/fielder fields.

@andrii-i
Copy link
Collaborator Author

CC-ing @dlqqq (as an author of the quote based on which this issue was created) to @ellisonbg comment above

@dlqqq
Copy link
Collaborator

dlqqq commented Oct 27, 2022

Renaming suggestions will be tracked in a separate issue here: #210

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants