Feature/add search functionality for models #393
Conversation
wkirdp
commented
Sep 21, 2023
wkirdp
commented
- Define filters for Billing Projects, Managed Groups, Workspaces and Accounts lists
- Use filters in BillingProjectList, ManagedGroupList, WorkspacesList, WorkspaceListByType, AccountList, AccountActiveList, and AccountInctiveList views.
- Add filter forms on billingproject_list, managedgroup_list, workspace_list and account_list templates
- Add tests
Add filter.py, define the filter for BillingProjects Modify billingproject_list.html to include the filter form Add BillingProjectFilter to BillingProjectList view.
Add filter to ManageGroupList view Add the FilterSet to managedgroup_list template Add tests
Add filter to WorkspaceList view and WorkspaceListByType view Add filter form on workspace_list template Add tests for WorkspaceList view and WorkspaceListByType view
Add AccountList filter to AccountList, AccountActiveList and AccountInactiveList views Add filter form to account_list template Add tests for AccountList, AccountActiveList and AccountInactiveList views
Codecov Report
@@ Coverage Diff @@
## main #393 +/- ##
========================================
Coverage 99.82% 99.82%
========================================
Files 119 121 +2
Lines 22335 22802 +467
========================================
+ Hits 22295 22762 +467
Misses 40 40
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| self.assertEqual(response.status_code, 200) | ||
| self.assertIn("table", response.context_data) | ||
| self.assertEqual(len(response.context_data["table"].rows), 2) | ||
|
|
There was a problem hiding this comment.
Once you have the AccountAdapter included, you'll want to add a test that checks filtering using the custom fields that a user has specified in the adapter. See the AccountAutocomplete tests for an example. This will requiring modifying the TestAccountAdapter in anvil_consortium_manager/tests/test_app/adapters.py
| self.assertEqual(response.status_code, 200) | ||
| self.assertIn("table", response.context_data) | ||
| self.assertEqual(len(response.context_data["table"].rows), 2) | ||
|
|
There was a problem hiding this comment.
Add a test for a user-specified workspace type - see the test_adapter method in this test case for an example. You'll want to test that it only finds the workspace of the correct type, not the workspace of the other type, even if both have a name that matches your querystring.
This custom form uses the crispy_form package's layout objects to manipulate the placement and apperance of forms. In particular, it displays the form fields on a single line, including the submit button, with floating labels. This takes up a smaller footprint and is less intrusive compared to the previous standard way of displaying the form.
|
@wkirdp I played around with the filter form to get something that is a little less intrusive. Please see the changes I made to |
Pass in query parameter instead of manually appending to the url
Modify tests to be testing more specific things (exact match, contains match, icontains match, multiple objects). For some views, also check that other objects are excluded when appropriate (eg that inactive accounts do not show up in the active account view filter).
This is the same form that was implemented for BillingProjectListFilter.
Missed this one when I was updating the other templates.
This lets the user customize the filterset_class used in the AccountList view. (Example: a project using this app wants to allow people to search on both the account email and the user name associated with the account.)
Add an AccountAdapterMixin, which sets the adapter attribute of the view class. It also contains reusable methods to get information from the adapter (e.g., the list class and the filterset class). Use this mixin for all of the AccountList views, which uses the list_filterset_class defined by whichever account adapter is set by the project settings file.
setup.cfg
Outdated
| @@ -35,6 +35,7 @@ install_requires = | |||
| google-auth >= 2.6 | |||
| fontawesomefree >= 6.1 | |||
| django-autocomplete-light >= 3.9 | |||
| django-filter > 2.0 | |||
There was a problem hiding this comment.
Don't think this version is correct
There was a problem hiding this comment.
@wkirdp Can you look into this? Did you mean > 23.0? >=23.0?
There was a problem hiding this comment.
@amstilp I pinned it to 23.2 and you said maybe not to pin to such a high version. Maybe just >2.0. Did I misunderstand? (After version 2.4, it jumped to 21.1, then 22.1 and 23.1)
There was a problem hiding this comment.
Sorry, I saw version 23.2 in requirements/base.txt and >2 in setup.cfg and thought it was unintentional. Ignore if that version makes sense.
There was a problem hiding this comment.
@wkirdp No, you didn't misunderstand. Best practice is to pin to the lowest working version. I did a couple tests with different versions of django-filter, and the tests fail with versions < 2.3.0. I switched the requirement in setup.cfg to require django-filter >= 2.3.0.
Tests fail with any version < 2.3.0.
Mostly this includes setting the PIP_NO_CACHE_DIR environment variable. I'm not sure why it was working in the other branch, or if it will definitely fix it here.
The previous commit used the working CI file, but I forgot to add the corresponding tox file. Hopefully this works.
|
CI issues seem to be intermittent and solved by retrying tox runs using the retry github action. The failures happen more often when there is no cache. It does not seem to be anything in the code. |
