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

Reset Make Filter #55

Merged
merged 10 commits into from
Nov 9, 2023
Merged

Reset Make Filter #55

merged 10 commits into from
Nov 9, 2023

Conversation

jdrco
Copy link
Contributor

@jdrco jdrco commented Nov 9, 2023

Describe your changes

This PR accomplishes the following:

  • Establishes reset for other filters
  • Implements the filter logic for the Make filter
  • Autofills the make filter with the last make filter applied

Issue ticket number and link

This PR addresses: #52

Checklist before requesting a review

  • I have performed a self-review of my code
  • Necessary java docs are present
  • Necessary UML diagrams are created
  • Necessary tests are written

Screenshots (if applicable)

@jdrco jdrco linked an issue Nov 9, 2023 that may be closed by this pull request
* @param filterClass The class of filter to reset.
*/
public void onFilterReset(Class<?> filterClass) {
for (Filter filter : appliedFilters) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we could just call something along the lines of appliedFilters.remove(filterinstance) no? Because appliedFilters is a Set not a List. See corresponding comment for what we would pass at the call location.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, below line 99 I believe we should have applyFilters(); being called

Copy link
Contributor Author

@jdrco jdrco Nov 9, 2023

Choose a reason for hiding this comment

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

See my response about passing this to the callback.
Doing a simple .remove could be possible if we store the MakeFilter in the MakeFilterFragment as an attribute and then pass that MakeFilter to the callback

*/
@Override
public void resetFilter() {
filterCallback.onFilterReset(MakeFilter.class);
Copy link
Contributor

Choose a reason for hiding this comment

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

filterCallback.onFilterReset(this);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this here is a MakeFilterFragment and not a MakeFilter

@jdrco jdrco changed the title Make reset filter Reset Make Filter Nov 9, 2023
owencooke and others added 4 commits November 8, 2023 19:17
* Working Espresso test for Add Item
Uses two utility functions:
1. Sets up MainActivity with a test user
2. Added a 'waitForView' method to allow waiitng for UI to load
---------

Co-authored-by: ldbonkowski <lbonkows@ualberta.ca>
Co-authored-by: ldbonkowski <77549591+ldbonkowski@users.noreply.github.com>
* Delete Item button

* PR changes
Copy link
Contributor

@owencooke owencooke left a comment

Choose a reason for hiding this comment

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

Looks good, once other comments are resolved should be good to go

@jdrco
Copy link
Contributor Author

jdrco commented Nov 9, 2023

@ldbonkowski @owencooke PR is updated

Copy link
Contributor

@ldbonkowski ldbonkowski left a comment

Choose a reason for hiding this comment

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

LGTM!

@jdrco jdrco merged commit d35dfe5 into main Nov 9, 2023
2 checks passed
@jdrco jdrco deleted the reset-filter branch November 9, 2023 18:32
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.

[SUBTASK] add reset to make filter
3 participants