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

Add Job Modal #744

Merged
merged 4 commits into from
May 22, 2024
Merged

Add Job Modal #744

merged 4 commits into from
May 22, 2024

Conversation

Diluka
Copy link
Contributor

@Diluka Diluka commented May 17, 2024

#426

  • only test example.ts (Express). I'm not familiar with other adaptors
  • jsoneditor's modal in modal need additional config, but I don't know how Modal Issue josdejong/jsoneditor#895
  • I only add enUS
  • You may need replace Icon to match theme

@felixmosh
Copy link
Owner

I would like to make several changes.

  1. move the add button to queues actions (3 dots)
  2. change json editor to somthing that will allow as to specify a schema (current json editor has redandant features)
  3. make all bull / bullmq add job options available (not only attemps & delay) via options json property

@Diluka
Copy link
Contributor Author

Diluka commented May 20, 2024

I would like to make several changes.

  1. move the add button to queues actions (3 dots)
  2. change json editor to somthing that will allow as to specify a schema (current json editor has redandant features)
  3. make all bull / bullmq add job options available (not only attemps & delay) via options json property
  1. good point
  2. any suggestions about the json editor? but there is no specific schema, because the data can be any form of json, even primitive values.
  3. I suggest just keep the two options for now. because the official dashboard only has the two options. Leave more advanced features to other people's PRs.
  4. could you please provide an icon for this button?

@felixmosh
Copy link
Owner

Sorry, I wasn't clear, I will make these changes.
Can you revert your latest 2 pushes?
(I've made some changes based on your PR.)
I will merge it, and apply my changes over it 🙏🏽

@Diluka
Copy link
Contributor Author

Diluka commented May 21, 2024

Sorry, I wasn't clear, I will make these changes. Can you revert your latest 2 pushes? (I've made some changes based on your PR.) I will merge it, and apply my changes over it 🙏🏽

I rebased the last commit(add to header) to current(add to ...).

You can continue to rebase, even if all my commits are removed, it doesn't matter.

@felixmosh
Copy link
Owner

Can you push them (so I will merge this PR?)

some progress :]
image

@Diluka
Copy link
Contributor Author

Diluka commented May 21, 2024

I rebased, so the old commit is lost. there is only new one in the branch.

If it's messed up, you can create a new branch and copy some of my codes. And close this one. it's OK.

@felixmosh
Copy link
Owner

OK, I'll check what can be done

@Diluka
Copy link
Contributor Author

Diluka commented May 21, 2024

image

Before my trial period ends, I’d better keep a screenshot of it. https://taskforce.sh

@felixmosh
Copy link
Owner

felixmosh commented May 21, 2024

This is so weird, what if you want to add other job option, you can't?
I prefer it to allow you to add a json, with a schema validation.

@Diluka
Copy link
Contributor Author

Diluka commented May 21, 2024

Maybe to prevent the queue from being broken? These two options are safe. If options are changed incorrectly(failParentOnFailure, lifo ...), the worker may throw an exception.

The above is my guess. In fact, when I debug the queue myself, I almost never pass the job options.

@Diluka
Copy link
Contributor Author

Diluka commented May 21, 2024

Can you push them (so I will merge this PR?)

some progress :] image

I'm curious how you know what type of job data should be? I remember that there is no type information saved on the queue.

@felixmosh
Copy link
Owner

Job data is from type any... so it won't validate it.
But job options, can be validated.

@felixmosh
Copy link
Owner

Progress :]
image
Schemas are auto generated

@felixmosh felixmosh merged commit bad7942 into felixmosh:master May 22, 2024
3 checks passed
@felixmosh
Copy link
Owner

Closes #266
Closes #425

@felixmosh
Copy link
Owner

Thank you so much for pushing this PR, it helped me to push this required feature to the lib 🙏🏽
Released in v5.18.0 🎊

@Diluka Diluka mentioned this pull request May 27, 2024
@snird
Copy link

snird commented May 30, 2024

This is an awesome feature.
I wanted to say thank you again for your great work @Diluka @felixmosh

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

Successfully merging this pull request may close these issues.

3 participants