CON-337 return all rooms when there is no location #506

MCFrank16 · 2019-10-11T13:53:55Z

Description

Type of change

Please select the relevant option

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Others (cosmetics, styling, improvements)
This change requires a documentation update

How Has This Been Tested?

Unit
Integration
End-to-end

How to Test

Give instructions on how reviewers can verify your work

Any background context you want to add

JIRA

CON-337

api/room/schema_query.py

joshuaocero · 2019-10-15T06:25:39Z

api/room/schema_query.py

@@ -280,7 +280,8 @@ def resolve_all_rooms(self, info, **kwargs):
    def resolve_all_remote_rooms(self, info, return_all=None):
        page_token = None
        filter = map_remote_room_location_to_filter()
-        location = 'all' if return_all else get_user_from_db().location
+        location = get_user_from_db().location
+        location = 'all' if not filter.get(location) or return_all else location


@MCFrank16 your else condition seems like repetition. You can get rid of it.

Let me check that @joshuaocero

@joshuaocero is there any situation that an admin can view all remote rooms while his location is available in the map filter function?

@MCFrank16 yes, if the admin passes the return_all field, they should be able to see all locations even if their location is available in the map filter.

joshuaocero · 2019-10-16T08:43:08Z

api/room/schema_query.py

@@ -280,7 +280,8 @@ def resolve_all_rooms(self, info, **kwargs):
    def resolve_all_remote_rooms(self, info, return_all=None):
        page_token = None
        filter = map_remote_room_location_to_filter()
-        location = 'all' if return_all else get_user_from_db().location
+        location = get_user_from_db().location
+        if not filter.get(location) or return_all: location = 'all'      


@MCFrank16 put the assignment on a separate line to allow for easy reading of your code.

ok... let me do it

joshuaocero

Please respond to those minor naming changes and we'll be good to go.

joshuaocero · 2019-10-17T09:58:30Z

fixtures/room/query_room_fixtures.py

        }
    }
+}
+
+all_remote_rooms_query2 = '''


@MCFrank16 please name this fixture appropriately. The variable name should be more descriptive of what the query is doing.

joshuaocero · 2019-10-17T09:59:02Z

fixtures/room/query_room_fixtures.py

+}
+'''
+
+all_remote_rooms_response2 = {


@MCFrank16 please name this fixture variable appropriately. The variable name should be more descriptive of what the response contains.

joshuaocero · 2019-10-17T10:00:28Z

tests/test_rooms/test_query_rooms.py

@@ -78,6 +80,17 @@ def test_calendar_list_function(self, mocked_method):
        get_google_api_calendar_list()
        assert mocked_method.called

+    def test_all_remote_rooms_with_true(self):


@MCFrank16 please change the grammar in the function name. test_all_remote_rooms_with_true makes no sense. Perhaps you could try test_all_remote_rooms_with_returnall_true.

joshuaocero · 2019-10-21T06:31:05Z

fixtures/room/query_room_fixtures.py

        }
    }
+}
+
+all_remote_rooms_query_with_returnAll_True = '''


Please look into your variable naming. Read the conventions -> https://www.python.org/dev/peps/pep-0008/#function-and-variable-names

joshuaocero · 2019-10-21T06:31:13Z

fixtures/room/query_room_fixtures.py

+}
+'''
+
+all_remote_rooms_response_with_returnAll_True = {


Please look into your variable naming. Read the conventions -> https://www.python.org/dev/peps/pep-0008/#function-and-variable-names

joshuaocero · 2019-10-21T06:31:24Z

tests/test_rooms/test_query_rooms.py

@@ -78,6 +80,17 @@ def test_calendar_list_function(self, mocked_method):
        get_google_api_calendar_list()
        assert mocked_method.called

+    def test_all_remote_rooms_with_argument_returnAll_true(self):


Please look into your variable naming. Read the conventions -> https://www.python.org/dev/peps/pep-0008/#function-and-variable-names

- return all rooms when the location is not available in the map filter [Delivers CON-337]

MCFrank16 added the Ready for Review label Oct 13, 2019

MCFrank16 requested review from mifeille, Ramadhan0 and joshuaocero October 13, 2019 11:36

MCFrank16 changed the title ~~CON-337-fix(remote-rooms): return all rooms when there is no location~~ CON-337 return all rooms when there is no location Oct 13, 2019

mifeille reviewed Oct 14, 2019

View reviewed changes

api/room/schema_query.py Show resolved Hide resolved

MCFrank16 force-pushed the bug/CON-337-return-all-rooms-without-filter branch from 77fe190 to 1e1f832 Compare October 14, 2019 13:13

mifeille approved these changes Oct 14, 2019

View reviewed changes

MCFrank16 added the To be merged This PR is ready to be merged, developer can start on a new story label Oct 14, 2019

mifeille removed the To be merged This PR is ready to be merged, developer can start on a new story label Oct 14, 2019

Ramadhan0 approved these changes Oct 14, 2019

View reviewed changes

joshuaocero suggested changes Oct 15, 2019

View reviewed changes

MCFrank16 force-pushed the bug/CON-337-return-all-rooms-without-filter branch 2 times, most recently from b73076d to 5b7fee2 Compare October 16, 2019 07:39

joshuaocero suggested changes Oct 16, 2019

View reviewed changes

MCFrank16 force-pushed the bug/CON-337-return-all-rooms-without-filter branch 11 times, most recently from 898c55b to 6b94245 Compare October 17, 2019 07:32

joshuaocero suggested changes Oct 17, 2019

View reviewed changes

MCFrank16 force-pushed the bug/CON-337-return-all-rooms-without-filter branch 2 times, most recently from fd18d12 to 038442e Compare October 17, 2019 10:18

MCFrank16 removed the Ready for Review label Oct 18, 2019

MCFrank16 added the To be merged This PR is ready to be merged, developer can start on a new story label Oct 18, 2019

joshuaocero suggested changes Oct 21, 2019

View reviewed changes

MCFrank16 force-pushed the bug/CON-337-return-all-rooms-without-filter branch 15 times, most recently from 5d357d5 to 4c6d3e4 Compare October 23, 2019 08:46

joshuaocero approved these changes Oct 24, 2019

View reviewed changes

MCFrank16 force-pushed the bug/CON-337-return-all-rooms-without-filter branch 7 times, most recently from 886fba8 to 98a958d Compare October 29, 2019 09:46

MCFrank16 force-pushed the bug/CON-337-return-all-rooms-without-filter branch from 98a958d to 71bd74c Compare November 25, 2019 12:53

CON-337-fix(remote-rooms): return all rooms when there is no location

f52245b

- return all rooms when the location is not available in the map filter [Delivers CON-337]

MCFrank16 force-pushed the bug/CON-337-return-all-rooms-without-filter branch from 71bd74c to f52245b Compare November 25, 2019 13:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CON-337 return all rooms when there is no location #506

CON-337 return all rooms when there is no location #506

MCFrank16 commented Oct 11, 2019 •

edited by mifeille

Loading

joshuaocero Oct 15, 2019

MCFrank16 Oct 15, 2019

MCFrank16 Oct 15, 2019

joshuaocero Oct 15, 2019

joshuaocero Oct 16, 2019

MCFrank16 Oct 16, 2019

joshuaocero left a comment

joshuaocero Oct 17, 2019

joshuaocero Oct 17, 2019

joshuaocero Oct 17, 2019

joshuaocero Oct 21, 2019

joshuaocero Oct 21, 2019

joshuaocero Oct 21, 2019

CON-337 return all rooms when there is no location #506

Are you sure you want to change the base?

CON-337 return all rooms when there is no location #506

Conversation

MCFrank16 commented Oct 11, 2019 • edited by mifeille Loading

Description

Type of change

How Has This Been Tested?

How to Test

Any background context you want to add

JIRA

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshuaocero left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MCFrank16 commented Oct 11, 2019 •

edited by mifeille

Loading