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

        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

        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

        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

        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 Ramadhan0, joshuaocero and mifeille 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

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

Uh oh!

Conversation

MCFrank16 commented Oct 11, 2019 • edited by mifeille Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

How Has This Been Tested?

How to Test

Any background context you want to add

JIRA

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joshuaocero left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

MCFrank16 commented Oct 11, 2019 •

edited by mifeille

Loading