diff --git a/CHANGELOG.md b/CHANGELOG.md index 1721baf..e393f65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,12 +1,28 @@ # Changelog -## [1.0.3a1](https://github.com/NeonGeckoCom/skill-user_settings/tree/1.0.3a1) (2023-08-07) +## [1.0.4a3](https://github.com/NeonGeckoCom/skill-user_settings/tree/1.0.4a3) (2023-12-16) -[Full Changelog](https://github.com/NeonGeckoCom/skill-user_settings/compare/1.0.2...1.0.3a1) +[Full Changelog](https://github.com/NeonGeckoCom/skill-user_settings/compare/1.0.4a2...1.0.4a3) **Merged pull requests:** -- Fix TTS Intent Language Bug [\#100](https://github.com/NeonGeckoCom/skill-user_settings/pull/100) ([NeonDaniel](https://github.com/NeonDaniel)) +- Update test dependencies to stable specs [\#104](https://github.com/NeonGeckoCom/skill-user_settings/pull/104) ([NeonDaniel](https://github.com/NeonDaniel)) + +## [1.0.4a2](https://github.com/NeonGeckoCom/skill-user_settings/tree/1.0.4a2) (2023-11-20) + +[Full Changelog](https://github.com/NeonGeckoCom/skill-user_settings/compare/1.0.4a1...1.0.4a2) + +**Merged pull requests:** + +- Feat catch unlikely long names [\#103](https://github.com/NeonGeckoCom/skill-user_settings/pull/103) ([NeonDaniel](https://github.com/NeonDaniel)) + +## [1.0.4a1](https://github.com/NeonGeckoCom/skill-user_settings/tree/1.0.4a1) (2023-11-02) + +[Full Changelog](https://github.com/NeonGeckoCom/skill-user_settings/compare/1.0.3...1.0.4a1) + +**Merged pull requests:** + +- Remove username example from README [\#102](https://github.com/NeonGeckoCom/skill-user_settings/pull/102) ([strugee](https://github.com/strugee)) diff --git a/README.md b/README.md index d49ebfc..201a19d 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,6 @@ Neon can help you control your user preference settings via this skill. * Tell me my name. * Tell me my first name. * Tell me my last name. -* Tell me my username. * Tell me my email address. diff --git a/__init__.py b/__init__.py index 903d4ba..49a800b 100644 --- a/__init__.py +++ b/__init__.py @@ -423,13 +423,13 @@ def handle_say_my_name(self, message: Message): return utterance = message.data.get("utterance") profile = get_user_prefs(message) - + # a 32-char username is a generated UID, not a real username + real_username = len(profile["user"]["username"]) < 32 if not any((profile["user"]["first_name"], profile["user"]["middle_name"], profile["user"]["last_name"], profile["user"]["preferred_name"], - profile["user"]["full_name"], - profile["user"]["username"])): + profile["user"]["full_name"])) and not real_username: # TODO: Use get_response to ask for the user's name self.speak_dialog( "name_not_known", @@ -680,11 +680,14 @@ def _on_close(message): .require("rx_name").build()) def handle_set_my_name(self, message: Message): """ - Handle a request to set a user's name + Handle a request to set a user's name. Some considerations for name + parsing can be found from w3c: + https://www.w3.org/International/questions/qa-personal-names :param message: Message associated with request """ if not self.neon_in_request(message): return + confirmed = True utterance = message.data.get("utterance") name = message.data.get("rx_setting") or message.data.get("rx_name") if self.voc_match(utterance, "first_name"): @@ -708,13 +711,23 @@ def handle_set_my_name(self, message: Message): user_profile = get_user_prefs(message)["user"] - if request: + # Catch an invalid intent match + if (request and len(name.split()) > 3) or len(name.split()) > 4: + LOG.warning(f"'{name}' does not look like a {request} name.") + confirmed = self.ask_yesno("name_confirm_change", + {"position": self.translate( + f"word_{request or 'name'}"), + "name": name}) + + if not confirmed: + self.speak_dialog("name_not_confirmed", private=True) + elif request: if name == user_profile[request]: self.speak_dialog( "name_not_changed", {"position": self.translate(f"word_{request}"), "name": name}, private=True) - else: + elif confirmed: name_parts = (name if request == n else user_profile.get(n) for n in ("first_name", "middle_name", "last_name")) @@ -1085,6 +1098,7 @@ def _get_name_parts(name: str, user_profile: dict) -> dict: "last_name": name_parts[2]} else: LOG.warning(f"Longer name than expected: {name}") + # TODO: Better logic here name = {"first_name": name_parts[0], "middle_name": name_parts[1], "last_name": " ".join(name_parts[2:])} diff --git a/locale/en-us/dialog/name_confirm_change.dialog b/locale/en-us/dialog/name_confirm_change.dialog new file mode 100644 index 0000000..72e6f46 --- /dev/null +++ b/locale/en-us/dialog/name_confirm_change.dialog @@ -0,0 +1 @@ +Would you like to update your {{position}} to {{name}}? \ No newline at end of file diff --git a/locale/en-us/dialog/name_not_confirmed.dialog b/locale/en-us/dialog/name_not_confirmed.dialog new file mode 100644 index 0000000..963faba --- /dev/null +++ b/locale/en-us/dialog/name_not_confirmed.dialog @@ -0,0 +1 @@ +Okay, no changes have been made. \ No newline at end of file diff --git a/requirements/test.txt b/requirements/test.txt new file mode 100644 index 0000000..42c48e0 --- /dev/null +++ b/requirements/test.txt @@ -0,0 +1 @@ +neon-minerva[padatious]~=0.1 \ No newline at end of file diff --git a/setup.py b/setup.py index 227844f..94700db 100644 --- a/setup.py +++ b/setup.py @@ -89,6 +89,7 @@ def find_resource_files(): url=f'https://github.com/NeonGeckoCom/{SKILL_NAME}', license='BSD-3-Clause', install_requires=get_requirements("requirements.txt"), + extras_require={"test": get_requirements("requirements/test.txt")}, author='Neongecko', author_email='developers@neon.ai', long_description=long_description, diff --git a/skill.json b/skill.json index a6edbae..c54222b 100644 --- a/skill.json +++ b/skill.json @@ -25,7 +25,6 @@ "Tell me my name.", "Tell me my first name.", "Tell me my last name.", - "Tell me my username.", "Tell me my email address.", "Where am I?", "My birthday is...", diff --git a/test/test_resources.yaml b/test/test_resources.yaml index bba2188..5863a82 100644 --- a/test/test_resources.yaml +++ b/test/test_resources.yaml @@ -138,6 +138,8 @@ dialog: - 'word_email_title' - 'date_format_already_set' - 'date_format_changed' + - 'name_confirm_change' + - 'name_not_confirmed' # regex entities, not necessarily filenames regex: - 'rx_language' diff --git a/test/test_skill.py b/test/test_skill.py index a671f68..843c3c4 100644 --- a/test/test_skill.py +++ b/test/test_skill.py @@ -25,30 +25,26 @@ # LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING # NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS # SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -import json -import os -import shutil + import unittest +from time import sleep + +import mock from copy import deepcopy from datetime import datetime -from os import mkdir -from os.path import dirname, join, exists from typing import Optional - -import mock from dateutil.tz import gettz from mock import Mock from mock.mock import call from neon_utils.user_utils import get_user_prefs from neon_utils.language_utils import SupportedLanguages -from ovos_utils.messagebus import FakeBus from ovos_bus_client import Message -from mycroft.skills.skill_loader import SkillLoader +from neon_minerva.tests.skill_unit_test_base import SkillTestCase -class TestSkill(unittest.TestCase): +class TestSkill(SkillTestCase): test_message = Message("test", {}, {"neon_in_request": True}) default_config = deepcopy(get_user_prefs()) default_config['location']['country'] = "United States" @@ -57,35 +53,10 @@ class TestSkill(unittest.TestCase): @mock.patch('neon_utils.language_utils.get_supported_languages') def setUpClass(cls, get_langs) -> None: get_langs.return_value = SupportedLanguages({'en'}, {'en'}, {'en'}) - # Define a directory to use for testing - cls.test_fs = join(dirname(__file__), "skill_fs") - if not exists(cls.test_fs): - mkdir(cls.test_fs) - os.environ["NEON_CONFIG_PATH"] = cls.test_fs - - bus = FakeBus() - bus.run_in_thread() - skill_loader = SkillLoader(bus, dirname(dirname(__file__))) - skill_loader.load() - cls.skill = skill_loader.instance - - # Override the configuration and fs paths to use the test directory - cls.skill.settings_write_path = cls.test_fs - cls.skill.file_system.path = cls.test_fs - cls.skill._init_settings() - cls.skill.initialize() - - # Override speak and speak_dialog to test passed arguments - cls.skill.speak = Mock() - cls.skill.speak_dialog = Mock() - - @classmethod - def tearDownClass(cls) -> None: - shutil.rmtree(cls.test_fs) + SkillTestCase.setUpClass() def setUp(self): - self.skill.speak.reset_mock() - self.skill.speak_dialog.reset_mock() + SkillTestCase.setUp(self) self.user_config = deepcopy(self.default_config) def test_00_skill_init(self): @@ -421,6 +392,7 @@ def _init_test_message(voc, location): # Change location same tz _init_test_message("location", "new york") + sleep(1) self.skill.handle_change_location_timezone(test_message) self.skill.ask_yesno.assert_called_once_with( "also_change_location_tz", {"type": "timezone", "new": "new york"}) @@ -441,6 +413,7 @@ def _init_test_message(voc, location): # Change tz same location _init_test_message("timezone", "phoenix") + sleep(1) self.skill.handle_change_location_timezone(test_message) self.skill.ask_yesno.assert_called_once_with( "also_change_location_tz", {"type": "location", "new": "phoenix"}) @@ -459,6 +432,7 @@ def _init_test_message(voc, location): # Change location and tz _init_test_message("location", "honolulu") + sleep(1) self.skill.handle_change_location_timezone(test_message) self.skill.ask_yesno.assert_called_once_with( "also_change_location_tz", {"type": "timezone", "new": "honolulu"}) @@ -486,6 +460,7 @@ def _init_test_message(voc, location): # Change tz and location _init_test_message("timezone", "phoenix") + sleep(1) self.skill.handle_change_location_timezone(test_message) self.skill.ask_yesno.assert_called_once_with( "also_change_location_tz", {"type": "location", "new": "phoenix"}) @@ -979,6 +954,74 @@ def test_handle_set_my_name(self): self.assertEqual(test_message.context["user_profiles"][0] ["user"]["preferred_name"], "Dan") + real_ask_yesno = self.skill.ask_yesno + self.skill.ask_yesno = Mock(return_value=False) + + # Test first name too long, unconfirmed + test_message.data['utterance'] = ("my first name is some misfired " + "intent match") + test_message.data["rx_setting"] = "some misfired intent match" + self.skill.handle_set_my_name(test_message) + self.skill.ask_yesno.assert_called_with("name_confirm_change", + {"position": "first name", + "name": test_message.data[ + "rx_setting"].title()}) + self.skill.speak_dialog.assert_called_with("name_not_confirmed", + private=True) + + # Test full name too long, unconfirmed + test_message.data['utterance'] = ("my name is some other misfired " + "intent match") + test_message.data["rx_setting"] = "some other misfired intent match" + self.skill.handle_set_my_name(test_message) + self.skill.ask_yesno.assert_called_with("name_confirm_change", + {"position": "name", + "name": test_message.data[ + "rx_setting"].title()}) + self.skill.speak_dialog.assert_called_with("name_not_confirmed", + private=True) + + self.skill.ask_yesno.return_value = True + # Test full name too long, confirmed + test_message.data['utterance'] = ("my name is José Eduardo Santos " + "Tavares Melo Silva") + test_message.data["rx_setting"] = "José Eduardo Santos Tavares Melo Silva" + self.skill.handle_set_my_name(test_message) + self.skill.ask_yesno.assert_called_with("name_confirm_change", + {"position": "name", + "name": test_message.data[ + "rx_setting"].title()}) + self.skill.speak_dialog.assert_called_with( + "name_set_full", + {"nick": test_message.context["user_profiles"][0]["user"] + ["preferred_name"], + "name": test_message.data["rx_setting"]}, private=True) + self.assertEqual(test_message.context["user_profiles"][0] + ["user"]["full_name"], test_message.data["rx_setting"]) + self.assertEqual(test_message.context["user_profiles"][0] + ["user"]["first_name"], "José") + self.assertEqual(test_message.context["user_profiles"][0] + ["user"]["middle_name"], "Eduardo") + self.assertEqual(test_message.context["user_profiles"][0] + ["user"]["last_name"], "Santos Tavares Melo Silva") + + # Test last name too long, confirmed no change + test_message.data['utterance'] = ("my last name is Santos " + "Tavares Melo Silva") + test_message.data["rx_setting"] = "Santos Tavares Melo Silva" + self.skill.handle_set_my_name(test_message) + self.skill.ask_yesno.assert_called_with("name_confirm_change", + {"position": "last name", + "name": test_message.data[ + "rx_setting"].title()}) + self.skill.speak_dialog.assert_called_with("name_not_changed", + {"position": "last name", + "name": test_message.data[ + "rx_setting"]}, + private=True) + + self.skill.ask_yesno = real_ask_yesno + def test_handle_say_my_language_settings(self): test_profile = self.user_config test_profile["user"]["username"] = "test_user" @@ -1334,6 +1377,7 @@ def test_get_name_parts(self): "full_name": "first-name middle last senior"}) def test_get_timezone_from_location(self): + sleep(1) name, offset = \ self.skill._get_timezone_from_location( self.skill._get_location_from_spoken_location("seattle")) @@ -1346,6 +1390,7 @@ def test_get_timezone_from_location(self): def test_get_location_from_spoken_location(self): # Test 'city' case + sleep(1) address = self.skill._get_location_from_spoken_location("seattle") self.assertEqual(address['address']['city'], "Seattle") self.assertEqual(address['address']['state'], "Washington") @@ -1354,6 +1399,7 @@ def test_get_location_from_spoken_location(self): self.assertIsInstance(address['lon'], str) # Test international case + sleep(1) address = self.skill._get_location_from_spoken_location("kyiv", "en-us") self.assertEqual(address['address']['city'], "Kyiv") @@ -1362,6 +1408,7 @@ def test_get_location_from_spoken_location(self): self.assertIsInstance(address['lon'], str) # Test 'town' case + sleep(1) address = self.skill._get_location_from_spoken_location( "kirkland washington") self.assertEqual(address['address']['city'], "Kirkland") @@ -1371,6 +1418,7 @@ def test_get_location_from_spoken_location(self): self.assertIsInstance(address['lon'], str) # Test 'village' case + sleep(1) address = self.skill._get_location_from_spoken_location( "orchard city colorado") self.assertEqual(address["address"]["city"], "Orchard City") diff --git a/version.py b/version.py index 052b4a4..39455a3 100644 --- a/version.py +++ b/version.py @@ -26,4 +26,4 @@ # NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS # SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -__version__ = "1.0.3" +__version__ = "1.0.4"