-
Notifications
You must be signed in to change notification settings - Fork 19
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
Corrected pagination for charges #241
Conversation
… merge rumby's modification on factories into my test branch
This will update the factory file in my branch so that I do not overrite existing factories by accident
… functions properly
…class for orgniazation_status but have been getting error message: factory not registered. Will come back to this later
…organization_status now passed!
…a comment in the status_type
Add in all the factories so that we don't get messed up from this point on More convinient to review other people's code
…hange before checking out akshay's unit tests
…th the built in timezone indicated in application.rb
…. Hopefully this works
…s configuration. Will create an issue
Merging goat branch to get the most recent update by my teammates, in case we have to edit common files again.
@blinblinblin can you clean up your PRs so only the relevant commits are in the PR? Its really hard to review when there are 30+ commits many of which have nothing to do with the PR and have already been merged. It also can easily lead to regressions when old code is accidentally merged. The easiest way to do this is to create a separate branch for each PR which you branch off the latest commit on |
@blinblinblin It looks like as of CanCan 1.4 that this filtering is done automatically for index methods. I would much prefer letting CanCan do all the filtering when returning objects. As you can see, doing it manually like this is really prone to breaking and requires whoever is writing index methods to know all the correct permissions for objects. If we put everything in Could you try upgrading CanCan to the latest version and see if it automatically filters charges? If that works, we can update other models to switch to CanCan in a future PR. |
@pkoenig10 I will close this PR and restart the bug fix in another branch. We will try to keep our submits clean from now on so that the commits look clean. Will look into cancan for this problem. |
According to #214 , there were problems with pagination on charges. @AkshayGoradia and I worked on this so that we cased on the users' role and show them the appropriate charges that they are supposed to see. @pkoenig10 Could you let us know if this is the desired behavior. Thanks!