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

Enable default Journal filter #306

Merged
merged 4 commits into from
Apr 14, 2016

Conversation

mbayopanda
Copy link
Collaborator

This PR add a filter option in journal controller at the client side, it's made by :

  • Adding a new service named JournalFilteringService
  • In this service we implement a function for filtering date in the 'yyyy-MM-dd' format

@sfount
Copy link
Contributor

sfount commented Apr 13, 2016

@jniles ▶️

pagination = new Pagination(vm.gridOptions, Transactions.list.data);
grouping = new Grouping(vm.gridOptions);
// enable filter option
vm.gridOptions.enableFiltering = true;
Copy link
Collaborator

Choose a reason for hiding this comment

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

All other journal services import the gridOptions to set up specific options. For example, this line in the pagination service configures external pagination. In the same way, sorting sets up sorting in this line.

This is a nice pattern developed by @sfount. I propose that you refactor filtering to take in gridOptions like this:

pagination = new Pagination(vm.gridOptions, Transactions.list.data);
filtering  = new Filtering(vm.gridOptions);

and set up the enableFiltering property there, like the other services.

@jniles
Copy link
Collaborator

jniles commented Apr 13, 2016

@mbayopanda - code LGTM. One small refactor will make this code look like the other services and be more maintainable. I'll merge it as soon as that lands.

Thanks!

@sfount sfount added this to the 2.x Process milestone Apr 13, 2016
@jniles
Copy link
Collaborator

jniles commented Apr 14, 2016

@mbayopanda - the code looks good to me!

@jniles jniles merged commit 893a6d9 into Third-Culture-Software:master Apr 14, 2016
@mbayopanda mbayopanda deleted the journal_filter branch September 14, 2016 08:55
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