I've worked a lot over the weekend when traveling back from Drupalcon Paris and today on integrating the good stuff from #527300: Tools modules (may have some reusable stuff) into our codebase. I've started with the suggestion approval screen, which was among the first requests among many translation teams.
Because Jose duplicated many of the filtering and displaying portions of the l10n_community module, I've worked on integrating the improvements to code. These in some cases resulted in cleaner code (filtering centralization, text formatting cleanup) and in some cases little new features (user filtering and limiting to less or more strings in the translation UI).
Changes included in the patch for the existing code include:
- centralized string formatting to l10n_community.module
- converted filters to arrays, centralized to l10n_community_build_filter_values()
- applied default size suggestions for project and search string fields
- added "submitted by" and "limit" filters to existing screens now that we have a screen which supplements their limitation of only looking at translations (this also makes filtering unified on the three screens, allowing for later switching among the screens to retain the filter, see #554144: Project information is not kept between View/Edit links and #220107: Allow search for user)
- fixed table class for filtering form and removed superfluous CSS styles
The new code adds a moderation screen with the (new) standard filter form (which only lacks the suggestion filter given that we only have suggestions here). You can mass approve or decline suggestions. I did not bring over the mass deletion from Jose's code, since I deem it to be too dangerous.
That is all. Big patch in itself already :)
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | mass-moderation-and-user-filter-fixed.patch | 39.76 KB | gábor hojtsy |
| mass-moderation-and-user-filter.patch | 39.05 KB | gábor hojtsy |
Comments
Comment #1
gábor hojtsyIt was tempting to add all these cool new things in but figured its best to add some test coverage first, so we know everything works. Indeed, the test coverage did find issues with how the code worked. Thankfully nothing in the update operations but rather in the listing code. That was all fixed. Issues I found while writing tests for this at #560132: Write tests are:
- We did not validate the limit argument properly, thus allowing for arbitrary limits to be set -- now limited to the allowed values only in the sanitization function -- note that this is due to how the data output from one form affects a query in another HTTP request; so we should sanitize values on all requests, regardless of our initial form validation
- The existing (unpatched) l10n_server DRUPAL-6--1 code had a bug with filtering for strings with no suggestions; we only matched for t.has_suggestion = 0 which assumed an actual row in the translation table, while strings lacking suggestions usually have no such row (AKA strings which never had a suggestion or translation before for the given language) -- solution is to use the same pattern as applied to untranslated strings where we also add has_suggestion IS NULL as a possible option
- The project filtering in the mass moderation screen was not using the right variables, was not actually working
- Context filtering UI was there, but was not implemented; also, context display was missing on the UI, now implemented
My automated tests now cover almost all the filtering operations in the form, including user lookup, substring search, project filtering, etc. The mass decline and approval actions are also validated to not touch unrelated strings but do touch all related strings (in case of approval for example, decline of other suggestions is validated).
Since all those tests pass, I'm confident that we can go ahead with this improvement and commit it! Wohoo!
Comment #2
gábor hojtsyAlso deployed to localize.drupal.org with much fanfare: http://localize.drupal.org/node/117