diff --git a/cms/sass/components/_select2.scss b/cms/sass/components/_select2.scss index 2675ff6bfc..fd395c855a 100644 --- a/cms/sass/components/_select2.scss +++ b/cms/sass/components/_select2.scss @@ -76,6 +76,12 @@ $s2-size: 20px; } } + .select2-search-choice-focus { + .select2-search-choice-close { + background-position: 0; + } + } + .select2-chosen { padding-bottom: $s2-space; @@ -105,7 +111,6 @@ $s2-size: 20px; } .select2-results { - .select2-highlighted, .select2-no-results, .select2-searching, .select2-ajax-error, @@ -113,6 +118,10 @@ $s2-size: 20px; background: $white; color: $warm-black; } + .select2-highlighted { + background: $light-grey; + color: $warm-black; + } } .select2-drop.select2-drop-above, diff --git a/doajtest/testbook/user_management/user_management.yml b/doajtest/testbook/user_management/user_management.yml index 55795719bd..cd9f2a55b1 100644 --- a/doajtest/testbook/user_management/user_management.yml +++ b/doajtest/testbook/user_management/user_management.yml @@ -88,6 +88,7 @@ tests: - You are taken to the new user form - The "api" role and "publisher" role are pre-filled in the Roles field. - The ID box is prefilled with an alphanumeric value + - There are no user attributes options available in the form - step: Click in the "Roles" field results: - A pull-down menu appears with all the allowed roles @@ -95,14 +96,13 @@ tests: results: - It is editable - Change the ID to something memorable - - step: Enter test values into each field in the form - - step: Click "Create User" + - step: Enter test values into each remaining field in the form + - step: Click "Register" results: - You are taken back to the /account - - You see a message Debug mode - url for verify is /account/reset/f1b6b123b5f34cd0aa81807a90a096f2 - - /account/reset/f1b6b123b5f34cd0aa81807a90a096f2 is a link and is clickable DO - NOT CLICK IT YET - - 'You see a message '' Account created for dom+test5@doaj.org. View Account: + - You see a message like "Debug mode - url for verify is ..." + - The verify URL in the message is a link and is clickable DO NOT CLICK IT YET + - 'You see a message '' Account created for [some email address]. View Account: /account/testingID''' - /account/testingID is a link and is clickable DO NOT CLICK IT YET - The ID (/testerID in the example above) matches the ID you chose on line 11 @@ -145,9 +145,26 @@ tests: - Your account page is shown - Your ID and email address are displayed - You can see your user roles but you cannot edit them + - You do not have any user attribute fields available to you - include: fragment: edit_account - + +- title: Edit your own user account (editor / associate editor) + context: + role: editor + setup: + - create an editor account with some attributes set + steps: + - step: Log in to your editor account + - step: Go to Settings under My Account + results: + - Your account page is shown + - Your ID and email address are displayed + - You can see your user roles but you cannot edit them + - You can see your user attributes but you cannot edit them + - include: + fragment: edit_account + - title: Edit your own user account (admin) context: role: admin @@ -157,6 +174,12 @@ tests: results: - Your account page is shown - Your ID and email address are displayed + - You can see your user roles and you can edit them + - You can see your user attributes and you can edit them + - step: Add one or more workflow attributes + - step: Add one or more language attributes + - step: Add one or more country attributes + - step: Add one or more free text tag attributes - include: fragment: edit_account - step: Log back in to your admin account and go to Settings @@ -180,6 +203,10 @@ tests: - The user's account page is displayed - A warning alerts you 'NOTE you are editing a user account that is not your own. Be careful!' + - step: Add one or more workflow attributes + - step: Add one or more language attributes + - step: Add one or more country attributes + - step: Add one or more free text tag attributes - include: fragment: edit_account - include: @@ -192,6 +219,7 @@ tests: - step: Put the username of the account you deleted into the search box results: - There are no results to display + - title: User account with journals/applications context: role: admin diff --git a/doajtest/unit/test_models.py b/doajtest/unit/test_models.py index 348eb1bd8b..91a983fdff 100644 --- a/doajtest/unit/test_models.py +++ b/doajtest/unit/test_models.py @@ -585,13 +585,15 @@ def test_08_iterate(self): def test_09_account(self): # Make a new account acc = models.Account.make_account(email='user@example.com', username='mrs_user', - roles=['api', 'associate_editor']) + roles=['api', 'associate_editor'], + attributes={constants.USER_ATTR__WORKFLOW: ["triage"]}) # Check the new user has the right roles assert acc.has_role('api') assert acc.has_role('associate_editor') assert not acc.has_role('admin') assert acc.marketing_consent is None + assert acc.has_attribute(constants.USER_ATTR__WORKFLOW, "triage") # check the api key has been generated assert acc.api_key is not None @@ -624,6 +626,58 @@ def test_09_account(self): acc2.save() assert acc2.api_key is not None + def test_09a_account_attributes(self): + acc = models.Account.make_account(email='user@example.com', username='mrs_user', + roles=['api', 'associate_editor'], + attributes={constants.USER_ATTR__WORKFLOW: ["triage"]}) + assert acc.has_attribute(constants.USER_ATTR__WORKFLOW, "triage") + + acc.add_attribute(constants.USER_ATTR__WORKFLOW, "quality") + acc.add_attribute(constants.USER_ATTR__LANGUAGE, "FR") + acc.add_attribute(constants.USER_ATTR__LANGUAGE, "EN") + acc.add_attribute(constants.USER_ATTR__COUNTRY, "DE") + acc.add_attribute(constants.USER_ATTR__COUNTRY, "ES") + acc.add_attribute(constants.USER_ATTR__TAG, "test") + + raw = acc.attributes + assert raw.get(constants.USER_ATTR__WORKFLOW) == ["triage", "quality"] + assert raw.get(constants.USER_ATTR__LANGUAGE) == ["FR", "EN"] + assert raw.get(constants.USER_ATTR__COUNTRY) == ["DE", "ES"] + assert raw.get(constants.USER_ATTR__TAG) == ["test"] + + assert acc.attribute_workflow == ["triage", "quality"] + assert acc.attribute_language == ["FR", "EN"] + assert acc.attribute_country == ["DE", "ES"] + assert acc.attribute_tag == ["test"] + + assert acc.has_attribute(constants.USER_ATTR__WORKFLOW, "triage") + assert acc.has_attribute(constants.USER_ATTR__WORKFLOW, "quality") + assert acc.has_attribute(constants.USER_ATTR__LANGUAGE, "FR") + assert acc.has_attribute(constants.USER_ATTR__LANGUAGE, "EN") + assert acc.has_attribute(constants.USER_ATTR__COUNTRY, "DE") + assert acc.has_attribute(constants.USER_ATTR__COUNTRY, "ES") + assert acc.has_attribute(constants.USER_ATTR__TAG, "test") + + assert acc.get_attributes(constants.USER_ATTR__WORKFLOW) == ["triage", "quality"] + assert acc.get_attributes(constants.USER_ATTR__LANGUAGE) == ["FR", "EN"] + assert acc.get_attributes(constants.USER_ATTR__COUNTRY) == ["DE", "ES"] + assert acc.get_attributes(constants.USER_ATTR__TAG) == ["test"] + + del acc.attributes + raw = acc.attributes + assert raw is None + + # try some error cases + with self.assertRaises(ValueError): + acc.add_attribute("whatever", "something") + + with self.assertRaises(ValueError): + acc.has_attribute("whatever", "something") + + with self.assertRaises(ValueError): + acc.get_attributes("whatever") + + def test_10_block(self): a = models.Article() a.save() diff --git a/portality/constants.py b/portality/constants.py index 11ef0b8f2c..a78431c61e 100644 --- a/portality/constants.py +++ b/portality/constants.py @@ -113,6 +113,27 @@ # TODO add ultra_bulk_delete and refactor view to use constants ROLE_ADMIN_REPORT_WITH_NOTES = "ultra_admin_reports_with_notes" # MUST start with ultra_ so that superusers don't gain +USER_ATTR__WORKFLOW = "workflow" +USER_ATTR__LANGUAGE = "language" +USER_ATTR__COUNTRY = "country" +USER_ATTR__TAG = "tag" + +USER_ATTR__ALL = [ + USER_ATTR__WORKFLOW, + USER_ATTR__LANGUAGE, + USER_ATTR__COUNTRY, + USER_ATTR__TAG +] + +EWF__TRIAGE = "Triage" +EWF__QUICK_FAIL = "Quick Fail" +EWF__QUALITY_REVIEW = "Quality Review" + +EWF__ALL_STAGES = [ + EWF__TRIAGE, + EWF__QUICK_FAIL, + EWF__QUALITY_REVIEW +] CRON_NEVER = {"month": "2", "day": "31", "day_of_week": "*", "hour": "*", "minute": "*"} diff --git a/portality/forms/validate.py b/portality/forms/validate.py index 228ca2b803..e3afc275c3 100644 --- a/portality/forms/validate.py +++ b/portality/forms/validate.py @@ -670,9 +670,13 @@ def __init__(self, message=None): def __call__(self, form, field): if field.data is not None and field.data != '': - check = get_currency_code(field.data, fail_if_not_found=True) - if check is None: - raise validators.ValidationError(self.message) + value = field.data + if not isinstance(value, list): + value = [value] + for v in value: + check = get_currency_code(v, fail_if_not_found=True) + if check is None: + raise validators.ValidationError(self.message) class CurrentISOLanguage(object): @@ -683,6 +687,10 @@ def __init__(self, message=None): def __call__(self, form, field): if field.data is not None and field.data != '': - check = isolang.find(field.data) - if check is None: - raise validators.ValidationError(self.message) + value = field.data + if not isinstance(value, list): + value = [value] + for v in value: + check = isolang.find(v) + if check is None: + raise validators.ValidationError(self.message) diff --git a/portality/migrate/4317_user_attributes/__init__.py b/portality/migrate/4317_user_attributes/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/portality/migrate/4317_user_attributes/apply_user_attributes.py b/portality/migrate/4317_user_attributes/apply_user_attributes.py new file mode 100644 index 0000000000..dde8fd1a38 --- /dev/null +++ b/portality/migrate/4317_user_attributes/apply_user_attributes.py @@ -0,0 +1,185 @@ +"""Migration script to add user attributes (country and language) based on Editor Group CSVs. + +Usage: + python -m portality.migrate.4317_user_attributes /path/to/countries.csv /path/to/languages.csv + +The first CSV should have two columns with headers: the first is the country name, the second the Editor Group name. +The second CSV should have two columns with headers: the first is the language name, the second the Editor Group name. + +For each row this script finds the EditorGroup by name, collects the accounts attached to the group +(maned, editor, associates) and schedules the appropriate ISO code to be added to each user's attributes +('country' or 'language'). All validations are done against the portality datasets (pycountry/isolang). + +The script collates all labels for each user and performs a single save per user at the end. +""" + +import csv +import sys +import argparse +from collections import defaultdict + +from portality.models.editors import EditorGroup +from portality.models.account import Account +from portality import datasets, constants +from portality.lib import isolang +from portality.core import app + + +def resolve_country_code(name: str): + """Return a 2-character country code for a given country name, or None if not found.""" + if not name: + return None + code = datasets.get_country_code(name, fail_if_not_found=True) + return code + + +def resolve_language_code(name: str): + """Return a 2-character language code (alpha2) for a given language name, or None if not found.""" + if not name: + return None + lang = isolang.find(name) + if not lang: + return None + # isolang.find returns a dict with 'alpha2' key + return lang.get('alpha2') + + +def read_pairs_from_csv(path): + """Read CSV and return list of (label, editor_group_name) pairs using the first two columns. + + Expects two columns with headers; will take the first and second header values. + """ + pairs = [] + with open(path, newline='') as fh: + reader = csv.DictReader(fh) + headers = reader.fieldnames + if not headers or len(headers) < 2: + raise ValueError(f"CSV at {path} must have at least two columns with headers") + col_label = headers[0] + col_group = headers[1] + for row in reader: + label = (row.get(col_label) or '').strip() + group = (row.get(col_group) or '').strip() + if label and group: + pairs.append((label, group)) + return pairs + + +def collate_attributes(country_pairs, language_pairs): + """Process pairs and collate desired attributes per account id. + + Returns mapping: account_id -> { 'country': set(...), 'language': set(...) } + """ + account_attrs = defaultdict(lambda: {'country': set(), 'language': set()}) + + def _collect_for_group(group_name, code, attr_type): + eg = EditorGroup.group_by_name(group_name) + if eg is None: + app.logger.warning(f"EditorGroup not found: '{group_name}'") + print(f"WARNING: EditorGroup not found: '{group_name}'") + return + + # collect maned, editor, associates + acct_ids = set() + if eg.maned: + acct_ids.add(eg.maned) + if eg.editor: + acct_ids.add(eg.editor) + for a in eg.associates or []: + acct_ids.add(a) + + for aid in acct_ids: + if not aid: + continue + account_attrs[aid][attr_type].add(code) + + # countries + for country_name, group_name in country_pairs: + code = resolve_country_code(country_name) + if code is None: + app.logger.warning(f"Unable to resolve country '{country_name}'") + print(f"WARNING: Unable to resolve country '{country_name}'") + continue + _collect_for_group(group_name, code.upper(), 'country') + + # languages + for lang_name, group_name in language_pairs: + code = resolve_language_code(lang_name) + if code is None: + app.logger.warning(f"Unable to resolve language '{lang_name}'") + print(f"WARNING: Unable to resolve language '{lang_name}'") + continue + _collect_for_group(group_name, code.upper(), 'language') + + return account_attrs + + +def apply_attributes(account_attrs, dry_run=False): + """Apply collated attributes to Account objects and save them once each. + + Returns a summary dict. + """ + summary = {'updated': 0, 'skipped_missing_account': 0} + for aid, attrs in account_attrs.items(): + acc = Account.pull(aid) + if acc is None: + summary['skipped_missing_account'] += 1 + app.logger.warning(f"Account not found: {aid}") + print(f"WARNING: Account not found: {aid}") + continue + + changed = False + # countries + for c in sorted(attrs.get('country', [])): + try: + if not acc.has_attribute(constants.USER_ATTR__COUNTRY, c): + acc.add_attribute(constants.USER_ATTR__COUNTRY, c) + changed = True + except Exception: + app.logger.exception(f"Failed to add country attribute '{c}' to account {aid}") + + for l in sorted(attrs.get('language', [])): + try: + if not acc.has_attribute(constants.USER_ATTR__LANGUAGE, l): + acc.add_attribute(constants.USER_ATTR__LANGUAGE, l) + changed = True + except Exception: + app.logger.exception(f"Failed to add language attribute '{l}' to account {aid}") + + if changed: + if dry_run: + print(f"DRY RUN: Would save account {aid} with new attributes: {attrs}") + else: + acc.save() + print(f"Saved account {aid} (added {len(attrs.get('country', []))} country(s), {len(attrs.get('language', []))} language(s))") + summary['updated'] += 1 + + return summary + + +def main(): + parser = argparse.ArgumentParser(description='Add user attributes from Editor Group CSVs') + parser.add_argument('countries_csv', help='CSV: first col country name, second col Editor Group name') + parser.add_argument('languages_csv', help='CSV: first col language name, second col Editor Group name') + parser.add_argument('--dry-run', action='store_true', help='Do not save changes, just report') + + args = parser.parse_args() + + country_pairs = read_pairs_from_csv(args.countries_csv) + language_pairs = read_pairs_from_csv(args.languages_csv) + + print(f"Read {len(country_pairs)} country rows and {len(language_pairs)} language rows") + + account_attrs = collate_attributes(country_pairs, language_pairs) + + print(f"Collated attributes for {len(account_attrs)} accounts") + + summary = apply_attributes(account_attrs, dry_run=args.dry_run) + + print("Done. Summary:") + print(summary) + + +if __name__ == '__main__': + main() + diff --git a/portality/models/account.py b/portality/models/account.py index 6441e88164..b75a9a6fbd 100644 --- a/portality/models/account.py +++ b/portality/models/account.py @@ -19,7 +19,7 @@ def __init__(self, **kwargs): super(Account, self).__init__(**kwargs) @classmethod - def make_account(cls, email, username=None, name=None, roles=None, associated_journal_ids=None): + def make_account(cls, email, username=None, name=None, roles=None, associated_journal_ids=None, attributes:dict[str, list]=None): if roles is None: roles = [] @@ -39,9 +39,17 @@ def make_account(cls, email, username=None, name=None, roles=None, associated_jo for role in roles: a.add_role(role) + for jid in associated_journal_ids: a.add_journal(jid) + if attributes is not None: + for attr_type, value in attributes.items(): + if not isinstance(value, list): + value = [value] + for v in value: + a.add_attribute(attr_type, v) + # New accounts don't have passwords set - create a reset token for password. reset_token = uuid.uuid4().hex # give them 14 days to create their first password if timeout not specified in config @@ -219,6 +227,56 @@ def set_role(self, role): role = [role] self.data["role"] = role + ############################### + ## user attributes + + @property + def attributes(self): + return self.data.get("attributes") + + @attributes.deleter + def attributes(self): + if "attributes" in self.data: + del self.data["attributes"] + + @property + def attribute_workflow(self): + return self.data.get("attributes", {}).get(constants.USER_ATTR__WORKFLOW, []) + + @property + def attribute_language(self): + return self.data.get("attributes", {}).get(constants.USER_ATTR__LANGUAGE, []) + + @property + def attribute_country(self): + return self.data.get("attributes", {}).get(constants.USER_ATTR__COUNTRY, []) + + @property + def attribute_tag(self): + return self.data.get("attributes", {}).get(constants.USER_ATTR__TAG, []) + + def add_attribute(self, attribute_type, value): + if attribute_type not in constants.USER_ATTR__ALL: + raise ValueError("Unknown user attribute type: {}".format(attribute_type)) + if "attributes" not in self.data: + self.data["attributes"] = {} + if attribute_type not in self.data["attributes"]: + self.data["attributes"][attribute_type] = [] + if value not in self.data["attributes"][attribute_type]: + self.data["attributes"][attribute_type].append(value) + + def has_attribute(self, attribute_type, value): + if attribute_type not in constants.USER_ATTR__ALL: + raise ValueError("Unknown user attribute type: {}".format(attribute_type)) + return value in self.data.get("attributes", {}).get(attribute_type, []) + + def get_attributes(self, attribute_type): + if attribute_type not in constants.USER_ATTR__ALL: + raise ValueError("Unknown user attribute type: {}".format(attribute_type)) + return self.data.get("attributes", {}).get(attribute_type, []) + + ####################### + def prep(self): self.data['last_updated'] = dates.now_str() @@ -326,3 +384,32 @@ def query(self): } } } + + @classmethod + def find_by_attributes(cls, attribute_types_and_values:list[tuple[str, str]], limit=1000): + q = AttributesQuery(attribute_types_and_values, limit) + return cls.object_query(q.query()) + + +class AttributesQuery: + def __init__(self, attribute_types_and_values, page_size): + self._tup = attribute_types_and_values + self._size = page_size + + def query(self): + musts = [] + for t, v in self._tup.items(): + if not isinstance(v, list): + v = [v] + f = {"terms": {f"attribute.{t}.exact": v}} + musts.append(f) + + q = { + "query": { + "bool": { + "must": musts + } + }, + "size": self._size + } + return q diff --git a/portality/templates-v2/_account/includes/_edit_form_js.html b/portality/templates-v2/_account/includes/_edit_form_js.html index bb6cf3ebfe..46f6c2386c 100644 --- a/portality/templates-v2/_account/includes/_edit_form_js.html +++ b/portality/templates-v2/_account/includes/_edit_form_js.html @@ -2,8 +2,101 @@