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

Keep datatable state in the browser history. #1986

Merged
merged 3 commits into from Jan 20, 2017
Merged

Keep datatable state in the browser history. #1986

merged 3 commits into from Jan 20, 2017

Conversation

nmb10
Copy link
Contributor

@nmb10 nmb10 commented Jan 12, 2017

Keep tasks datatable state in the browser history.

Description

Allows to refresh page without tasks datatable state missing. Also added abilities to add filtered/ordered tasks datatable to bookmarks and links exchange.

Motivation and Context

Sometimes we need to add filtered table with tasks to bookmarks or refresh the page.

Have you tested this? If so, how?

Selenium tests added for entries amount, order and filter (I used selenium + phantomjs driver because I needed 'wait for elements' feature which is too complicated for phantom.js)

@mention-bot
Copy link

@nmb10, thanks for your PR! By analyzing the history of the files in this pull request, we identified @daveFNbuck, @Tarrasch and @riga to be potential reviewers.

Copy link
Contributor

@Tarrasch Tarrasch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's a "datatable"? :)

Allows to refresh page without tasks datatable state missing

Butif we keep the state after a refresh, doesn't that kill the purpose of refreshing? I mean I want to see the new latest values. No?

I'm not sure I fully understand the part about

Also added abilities to add filtered/ordered tasks datatable to bookmarks and links exchange.

Did you mean you implement so that you can send urls like http://myluigi.com:8082/q=MyTask???

To clarify what's done here, can you add any screenshots or example urls to this PR to clarify what it adds? If it's not too much work.


By the way, super big kudos for adding test cases. :)

start: 0,
time: new Date().getTime(),
columns: [
{visible: true, search: {}},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are the 6 rows here? Can you comment on explaining this magic?

@nmb10
Copy link
Contributor Author

nmb10 commented Jan 13, 2017

Hi, @Tarrasch

What's a "datatable"? :)

It's js library you used to display table with tasks :) - https://datatables.net/

Butif we keep the state after a refresh, doesn't that kill the purpose of refreshing? I mean I want to see the new latest values. No?

No. Values (tasks) are not stored. History keeps order, filter and amount of entries on page (Entries select box value).

I'm not sure I fully understand the part about

An example: you selected to display 50 entries on page, you filtered by 'Uber' and ordered by name. Then you refreshed the page. After page refresh entries, filter and order stay the same (50, 'Uber', name).

Did you mean you implement so that you can send urls like http://myluigi.com:8082/q=MyTask???

Yes. This is copy-paste from local instance - http://127.0.0.1:8082/static/visualiser/index.html?search__search=Uber&order=1,asc&length=10

To clarify what's done here, can you add any screenshots or example urls to this PR to clarify what it adds? If it's not too much work.

http://127.0.0.1:8082/static/visualiser/index.html?search__search=Uber&order=1,asc&length=50 . But usually you do not need to construct such an url - just copy-paste it from browser.

What are the 6 rows here? Can you comment on explaining this magic?

Ok.

@Tarrasch
Copy link
Contributor

Ok. It's all clear to me now. I've wanted this thing for like forever. But I let someone else do the code review. :)

@Tarrasch
Copy link
Contributor

@daveFNbuck, can you review this? :)

@daveFNbuck
Copy link
Contributor

Looks good to me. Would be nice if we also saved the state of the graph buttons for easy sharing.

@Tarrasch Tarrasch merged commit 6e549ef into spotify:master Jan 20, 2017
@Tarrasch
Copy link
Contributor

@nmb10, thanks a lot, I'll try this out soon. :)

@nmb10
Copy link
Contributor Author

nmb10 commented Jan 20, 2017

Would be nice if we also saved the state of the graph buttons for easy sharing.

I'll check that.

kreczko pushed a commit to kreczko/luigi that referenced this pull request Mar 28, 2017
Allows to refresh page without tasks datatable state missing. Also added abilities to add filtered/ordered tasks datatable to bookmarks and links exchange.

In other words, you should now be able to share "what you are seeing" in the scheduler UI by sharing the URL in the browser.
This was referenced Jun 29, 2022
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.

None yet

4 participants