-
Notifications
You must be signed in to change notification settings - Fork 74
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
NEW: Advanced jobs admin UI. #307
NEW: Advanced jobs admin UI. #307
Conversation
44c4e28
to
b6fea5d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look great, you'll need to rebase/retarget this to 4
since it contains new API
/** | ||
* @return string | ||
*/ | ||
public function getImplementationSummary() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of the word 'summary' is a little weird, perhaps rename it to getTrimmedImplementation()
or similar?. I'd also add example to show that its trimming off the front of the namespace
README.md
Outdated
@@ -819,6 +819,17 @@ As a consequence, the work might end up being very fragmented and each chunk may | |||
|
|||
Some projects do not mind this however, so this solution may still be quite suitable. | |||
|
|||
## Jobs admin UI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
## Jobs admin UI | |
## Enabling advanced admin UI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about 'Enabling extended admin UI' instead of 'Advanced'? Even though this is the readme, we try to avoid advanced wording coupled in UI contexts where possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also comb through and add translation calls for all the new language strings?
db8636c
to
e5a655c
Compare
e5a655c
to
0cae034
Compare
@emteknetnz @Cheddam Rebased on |
@mfendeksilverstripe I tried testing this locally though it didn't seem to activate the filterable headers. I added the yml and installed the suggested module and ran /dev/build?flush It's possible that it's related to merge conflicts after merging #314 - could you rebase this and see if it fixes things up? |
0cae034
to
b680f48
Compare
@emteknetnz I rebased the branch. Also, I tested it on my local and it seems to be working fine. Have you enabled the UI on your local? It's disabled by default so you need to add following config to your YAML:
|
@mfendeksilverstripe yeah I've required I have included the following config and run /dev/build?flush Symbiote\QueuedJobs\Controllers\QueuedJobsAdmin:
advanced_admin_ui: true Also looks like you'll need to do another rebase (sorry) after I merged another one of your queuedjob prs |
b680f48
to
ed8bfc2
Compare
@emteknetnz I have rebased the branch and tested this feature on a vanilla install. It seems to be working fine. Here is my project config Note that I replaced the queued jobs module folder with this branch manually. I recommend checking the version of the rich filter header module and also the following setting: This setting should be automatically set by the rich filter header module. |
^ @emteknetnz Did you happen to get anywhere with this? Looks like a great addition and I'd be keen to see it merged in (noting there's a new conflict already heh) |
ed8bfc2
to
a658779
Compare
@madmatt I've rebased the branch so it should be easy to merge in now. |
@mfendeksilverstripe I must be missing something? I've included this PR, Here's mysite.yml + composer.json
|
Hey @emteknetnz here is a step by step guide which I used to test this feature on a vanilla install. Hopefully, this helps :) 1 -
7 -
9 -
12 - This is what I see when I navigate to the Queued jobs admin: cc: @madmatt |
Nope I'm still not seeing it. The steps you've given are basically the same as the sample composer.json and mysite.yml I provided above. I've also tried ^2 and dev-master of silverstripe-terraformers/gridfield-rich-filter-header though it made no difference Is there some extra config missing in either your .env file of vagrantfile? |
@emteknetnz I don't think my env file is related to this but here it is just for reference:
I don't have anything out of ordinary in the vagrant file either. I'll try to add a unit test which would validate that the UI is loading properly. |
35db45a
to
4b7af85
Compare
Hey @emteknetnz I've added unit tests to showcase that the new UI is working as expected. I recommend running the test suite on your local setup and see if it passes. |
0192076
to
ee8d906
Compare
ee8d906
to
62457d7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies, looking again at this with fresh eyes I'm really not sure that we should be essentially baking an third party optional module into queuedjobs. I feel like things should be implemented in a bit more of an "extendable" fashion.
Most core maintainers won't have merge access on the terraformers account. It seems like if we're going to integrate this module to this degree we'd probably need everything on the Silverstripe account. However that goes against the idea that functionality like this would be better maintained outside of the core team
It also seems like you could simply use the existing $this->extend('updateEditForm', $form);
hook within the terraformers module to to do everything you need to ... does this seem correct?
I'm happy with linking to the module within docs. Not so sure about suggest
in composer.json, does that still work in composer 2?
Thanks for clarification, I'll close this PR then. |
Advanced jobs admin UI
Optional advanced UI for jobs admin. Needs to be enabled via configuration API.
Problem
Finding specific jobs in the Jobs admin is a chore, developers often find themselves scrolling through multiple pages. Finding the right jobs is sometimes impossible as the filter criteria input is quite limited. Using Text fields for input is often tedious.
Solution
Improved UI. More filters, better input fields, more useful information in the columns.
Notes
Feature 1 - Improved Jobs admin UI
Related issues
#308