Feature: Implement Map for Locating Events #102

Zafouche · 2024-10-27T14:32:38Z

Description:

This PR introduces a map feature within the app, allowing users to view event locations. The changes include implementing a map interface, adding navigation to the map screen, setting up proper initialization for the event repository and view model, and configuring the API key injection process. The map currently displays all events, as I am waiting for the integration of event-saving logic.

Motivation and context:

This feature enhances the app's usability by providing a visual representation of events' locations, making it possible for users to locate the events of interest. Initially, the map displays all events; however, what still needs to be done is to show the saved events and those relevant to the user (e.g., nearby events), and further define the logic of color gradients and specific views.
The Google Maps API key setup is necessary for the functionality of the map.
The implementation of a proper event repository and view model initialization is needed for consistency and good behavior.

Changes:

Map Implementation: Created a simple map screen displaying all event locations.
Navigation Logic: Integrated navigation flow to access the map screen from the Home Screen.
Event Repository & ViewModel Initialization: Set up initialization processes for handling event data.
API Key Configuration: Added API key management in the Android manifest, utilizing a repository secret.
Tests: Fixed HomeScreen tests that used a callback function through the composable parameters, which is fundamentally not a proper way of testing.

Edit 1:

Added a custom pin point that refers to a "favorited" event. This custom pin point will be used for the
Added a mock list of an arbitrary saved event list. This will simply be replaced by the list of the saved events by the user when it is available (waiting for another PR)
Changed the color of the pin points that correspond to all the events to blue. This will later on (when more events are added to our database) be refactored to only take events that are in the very near future.

Edit 2:

Added time-until-event information: Each event marker now displays a countdown (in days and hours) until the event occurs.
Updated our Firestore database's events with actual dates and locations (so that we can straight away when loading the map some events around EPFL, instead of having them in the Golf of Guinea).

Edit 3:

Added simple tests for Map.kt: a display test and a "go back" button test.
Added tests to validate the init function of EentRepositoryFirestore.

What's left:

Show the saved events. This is done to an extend, with a mock list. This will be factored out and the actual data will be fetched from the view model when the saved event implementation (in the view model) is merged into main, which is currently another PR.
Show the events relevant to the user (e.g., nearby events). This would included the usage of sensors ; it will require to implement a permission request to the user, and perhaps will have to wait until the "user settings" are implemented.
Create a gradient of color to see the importance/date of the event (for example if it is tomorrow). This will also be done when the saved event implementation makes it into the main branch and we discuss how we want to customize our markers now that I have displayed here how a custom marker or the color can be changed.
Full End2End testing

How has this been tested?

Simple display testing
Simple button click testing

Other tests: Tested the init function I had to add to the Event repository.

Still left

Global End2End testing. (different task later on since this requires testing the MainActivity)

These changes simply introduce the usage of Google maps to display the events that are fetched through the event view model. No color yet.

- Added an init function in the EventRepository interface - Implemented the init function in the event repository 'EventRepositoryFirestore' - Modified the init { } block in EventListViewModel to also into account the initialization of its repository

- Added the map composable under the home route in the NavGraph. - Added initialization of the event repository and view model in the main activity (might not be useful later) - Added the right callback for the onClick field of the floating action button in the home screen.

This addition was needed because the EventRepository interface now has an init function.

This addition in the requestAll function of FirestoreReferenceList.kt fixes the crash that occurs every time the app is launched.

This is the API key required to use google maps. It should be defined in local.properties.

…ions Updated the FloatingActionButton icon in Home.kt to use Icons.Filled.Place instead of Icons.Filled.Add. This corresponds better the to the map screen.

A callback parameter was used to test the HomeScreen. This has been factored out and the tests have been corrected.

Zafouche · 2024-10-28T14:37:02Z

Since I need @Aurelien9Code's changes that implement the favorite event mechanism, e.g. "save event", I will work with an arbitrary saved event for all users, and refactor this when his PR makes it into our main branch successfully.

- Added a custom pin point that refers to a "favorited" event - Added a mock list of an arbitrary saved event list. This will simply be replaced by the list of the saved events by the user when it is available (waiting for another PR) - Changed the color of the pin points that correspond to all the events to blue. This will later on be refactored to only take events that are in the very near future.

Zafouche · 2024-10-28T22:13:36Z

Additional changes

In the meantime, here are additional changes I have made:

Added a custom pin point that refers to a "favorited" event. This custom pin point will be used for the
Added a mock list of an arbitrary saved event list. This will simply be replaced by the list of the saved events by the user when it is available (waiting for another PR)
Changed the color of the pin points that correspond to all the events to blue. This will later on (when more events are added to our database) be refactored to only take events that are in the very near future.

Screenshot

Here is what the custom pin point looks like. We'll have to decide of how we want our pin point to look for the saved events. This one is used in the meantime to showcase the difference between saved events' pin points and other non-favorited events in the near future.

- Added the "timeUntilEvent" function to calculate time remaining time until an event occurs, displayed in days and hours. - Updated marker snippet to show countdown information in front of the event description. - Added condition check to display only future events. This provides users with a clear indication of how much time remains until each event, improving the usability of the map.

Zafouche · 2024-10-29T13:39:36Z

Additional changes:

Added time-until-event information: Each event marker now displays a countdown (in days and hours) until the event occurs.
Updated our Firestore database's events with actual dates and locations (so that we can straight away when loading the map some events around EPFL, instead of having them in the Golf of Guinea).
Added simple tests for Map.kt: a display test and a "go back" button test.
Added tests to validate the init function of EentRepositoryFirestore.

This improves code readability.

…ources

- Added a test that checks that the main components of the MapScreen composable are displayed. - Added a test that checks that the "go back" button successfully triggers the navigation back.

Added tests to validate the init function. The choice of making these tests separate has been made, as, if successful according to SonarCloud standards, they will serve as models for the other repositories' tests.

Zafouche · 2024-10-29T21:51:49Z

@oskar-codes Good news, after quite a bit of research for testing, I have figured out how to cover the init functions of our repositories! Have a look at this commit.

This required me to combine the usage of Robolectric, like it was done in the Swent bootcamp for testing the ToDo repository, and the ArgumentCaptor class. ArgumentCaptor allows us to capture an argument passed to a method to inspect it. You can quickly read about it in the official documentation, or in a more guiding article.
Ultimately, this stackoverflow question helped implement it, which was fairly easy.

First, you define the captor like this:

private lateinit var authStateListenerCaptor: ArgumentCaptor<FirebaseAuth.AuthStateListener>

Then you use .capture() call on the captor when adding an auth state listener, e.g.

verify(auth).addAuthStateListener(authStateListenerCaptor.capture())

And the state listener can then be fetched through the captor with a simple .value, e.g.

authStateListenerCaptor.value.onAuthStateChanged(auth)

I have tested the two conditions, e.g. if the user is authenticated and if the user isn't authenticated. And indeed, SonarCloud is happy about this and shows that the init function is fully covered!
Now we finally know how to test the auth state listener.

- Added a check that verifies that the button is displayed and clickable - Added a check that verifies that the icon is displayed and clickable

This prevents initializing an actual event view model and repository in BottomNavigationTest.

sonarqubecloud · 2024-10-29T22:27:29Z

Quality Gate failed

Failed conditions
69.7% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

Zafouche · 2024-10-29T22:42:16Z

More tests will come tomorrow. Will open this PR for review for more detailed opinions.

AlexeiThornber

Overall, very nice additions to Unio @Zafouche, great work 💪 !
Some domains are still lacking, especially e2e testing, but that is something that should be done for the entire projetct for milestone 2 :/.
Try to separate the funtionality elements with the UI elements of your code, typically helper funtions can go in the ViewModel instead of the View.
I tried testing your PR on my device but google maps was not loading. I believe that is because I do not have the API keys and I forgot how to retrieve it 😓 .
Anyhow I'll trust you on this one and approve your PR even though there are some minor issues that can be worked on. Very solid work 🚀 .

AlexeiThornber · 2024-10-30T09:23:27Z

app/src/main/java/com/android/unio/ui/map/Map.kt

+@Composable
+fun EventMap(pd: PaddingValues, eventListViewModel: EventListViewModel) {
+  val cameraPositionState = rememberCameraPositionState {
+    position = CameraPosition.fromLatLngZoom(LatLng(46.518831258, 6.559331096), 10f)


You could possibly put this hardcoded location into a constant. @Aurelien9Code provided a new manager for all backend strings, we could possibly use this for hardcoded values too, but this is very minor.

These are Doubles but yes, they can at least be put in constants. In the future we should replace this with the user's current position.

AlexeiThornber · 2024-10-30T09:26:40Z

app/src/main/java/com/android/unio/ui/map/Map.kt

+  val events = eventListViewModel.events.collectAsState()
+
+  /** Mock arbitrary saved event for all users. */
+  val arbitrarySavedEvents: List<Event> =


Avoid mocking in the main code. But then again because this feature is still in devellopement, it is normal to do so. Especially as you explained in your PR, the saved events feature is not yet implemented.

Yeah, still waiting on the feature to make it into the main branch.

AlexeiThornber · 2024-10-30T09:31:09Z

app/src/main/java/com/android/unio/ui/map/Map.kt

+      cameraPositionState = cameraPositionState) {
+        // Display saved events
+        arbitrarySavedEvents.forEach { event ->
+          if (event.date.toDate() > Calendar.getInstance().time) {


I believe it's best practise to have all the functionality of the code in the viewModel, the UI (or View) should only be for displaying elements. Consider adding a checkEventHasPassed() function in the viewModel, this will improve readability. But I am nit picking here as I am the first one guilty of adding functionality in the View 😓.

You're right, but that would mean deleting old events. I think it will be necessary to keep old events in the database up to some "expiration date" and remove them when that "expiration date", as we potentially wanted to add some social features for past events, e.g. pictures or simple comments and whatnot.
Deleting old events is caching duty, so I think it's fair to say this is just display logic, don't you agree?

Yes absolutely. As I said i was nit picking and this is completly fine as it's done right now.

AlexeiThornber · 2024-10-30T09:34:09Z

app/src/main/java/com/android/unio/ui/map/Map.kt

+ * @param eventTimestamp the timestamp of the event
+ * @return a string giving information about the time until the event occurs
+ */
+fun timeUntilEvent(eventTimestamp: Timestamp): String {


This too I believe could be defined in the viewModel instead of the View for the reasons mentioned above.

Since the time counter is (at least for now) solely for informational purpose, I'd argue we keep it here.

AlexeiThornber · 2024-10-30T09:54:53Z

app/src/main/java/com/android/unio/model/event/EventRepositoryFirestore.kt

+    Firebase.auth.addAuthStateListener {
+      if (it.currentUser != null) {
+        onSuccess()
+      }


Consider adding some error handling if the user is null. Typically an onFailure() callback.

I would argue error handling shouldn't be done in this initializing function, as the repository shouldn't work if there is no user, as to not make unnecessary queries.
However I agree that we should at least Log it, e.g. have some failure functionality
Also I don't see what the onFailure function would do, except logging. In that case we can simply add an else Log.e:
If there is a user, we can loadEvents(), e.g. onSuccess call.
And if there is no user, we don't make any queries.

Do you have something additional in mind?

Zafouche · 2024-10-30T10:46:54Z

Overall, very nice additions to Unio @Zafouche, great work 💪 !

Thank you for your review @AlouchLaBouche ! All good points

Some domains are still lacking, especially e2e testing, but that is something that should be done for the entire projetct for milestone 2 :/.

I agree! I plan on writing e2e testing today, however I don't really see what could be done here, except clicking on markers.
Other than that a global e2e test would indeed cover the navigation in and out of the map screen, in addition to the tests that I made for the button click navigation and the go back button navigation.

Try to separate the funtionality elements with the UI elements of your code, typically helper funtions can go in the ViewModel instead of the View.

Totally! As you can see in my responses to your review comments, some are still ambiguous and depends on what comes next, so I appreciate more opinions and discussions.

I tried testing your PR on my device but google maps was not loading. I believe that is because I do not have the API keys and I forgot how to retrieve it 😓 . Anyhow I'll trust you on this one and approve your PR even though there are some minor issues that can be worked on. Very solid work 🚀 .

Yes, you should set up your API key in local.properties, like we did in the bootcamp. I suspected the secret only works for apks, you just confirmed it.

Zafouche · 2024-10-31T14:53:27Z

@AlouchLaBouche Are we happy to merge this?
I have thought about the e2e tests, and they will be implemented in a different PR, as an e2e test is a global test for the whole app.

AlexeiThornber · 2024-10-31T16:12:45Z

Yup, all seems to be in order 👍. E2E tests will be worked on next sprint I believe as they are essential to be done before milestone 2.

Zafouche added 10 commits October 25, 2024 21:18

feat(map): implement simple map showing event markers

d56fa26

These changes simply introduce the usage of Google maps to display the events that are fetched through the event view model. No color yet.

chore(test): adjust ScreenDisplayingTest.kt to new MapScreen parameter

6e07b7e

style(ktfmt): format files using ktfmtFormat

1fcfa86

chore(event-mock): add init implementation in EventRepositoryMock

3b19846

This addition was needed because the EventRepository interface now has an init function.

fix(reference-list): add empty edge case that caused crashes

b532ef4

This addition in the requestAll function of FirestoreReferenceList.kt fixes the crash that occurs every time the app is launched.

chore(main): add usage of the global event view model for MapScreen

4c0eb5c

chore(android-manifest): add Google Maps API key meta-data

ea92fbf

This is the API key required to use google maps. It should be defined in local.properties.

chore(home): replace FAB icon to represent map screen for event locat…

55579e7

…ions Updated the FloatingActionButton icon in Home.kt to use Icons.Filled.Place instead of Icons.Filled.Add. This corresponds better the to the map screen.

Zafouche linked an issue Oct 27, 2024 that may be closed by this pull request

Create map to locate events #101

Closed

fix(test): fix failing tests in EventListOverviewTest.kt

d0b29df

A callback parameter was used to test the HomeScreen. This has been factored out and the tests have been corrected.

Zafouche mentioned this pull request Oct 28, 2024

Feature/event list v3 #93

Merged

Zafouche added 5 commits October 29, 2024 14:56

refactor(map): extract marker display logic into helper function

4a5ac3d

This improves code readability.

Merge branch 'main' into feature/map-event-locations

5f3baa0

refactor(map): replace hardcoded strings in MapScreen with string res…

471fc6e

…ources

test(map): add simple display test and button test

583c436

- Added a test that checks that the main components of the MapScreen composable are displayed. - Added a test that checks that the "go back" button successfully triggers the navigation back.

test: add unit tests for EventRepositoryFirestore init function

a715143

Added tests to validate the init function. The choice of making these tests separate has been made, as, if successful according to SonarCloud standards, they will serve as models for the other repositories' tests.

Zafouche added 3 commits October 29, 2024 23:06

test(home): improve map FAB button tests

ec556cb

- Added a check that verifies that the button is displayed and clickable - Added a check that verifies that the icon is displayed and clickable

test(bottom-nav): adjust test to use a mocked repository and view model

3263958

This prevents initializing an actual event view model and repository in BottomNavigationTest.

style(ktfmt): format files using ktfmtFormat

09ee17b

Zafouche marked this pull request as ready for review October 29, 2024 22:42

Zafouche changed the title ~~Feature: Implement Map View for Locating Saved and Nearby Events~~ Feature: Implement Map for Locating Events Oct 30, 2024

AlexeiThornber approved these changes Oct 30, 2024

View reviewed changes

Zafouche merged commit 32a9daa into main Oct 31, 2024
1 of 2 checks passed

Zafouche deleted the feature/map-event-locations branch October 31, 2024 16:43

Feature: Implement Map for Locating Events #102

Feature: Implement Map for Locating Events #102

Uh oh!

Conversation

Zafouche commented Oct 27, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Motivation and context:

Changes:

Edit 1:

Edit 2:

Edit 3:

What's left:

How has this been tested?

Still left

Uh oh!

Zafouche commented Oct 28, 2024

Uh oh!

Zafouche commented Oct 28, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Additional changes

Screenshot

Uh oh!

Zafouche commented Oct 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Additional changes:

Uh oh!

Zafouche commented Oct 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sonarqubecloud bot commented Oct 29, 2024

Quality Gate failed

Uh oh!

Zafouche commented Oct 29, 2024

Uh oh!

AlexeiThornber left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zafouche Oct 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Zafouche Oct 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Zafouche Oct 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zafouche commented Oct 30, 2024

Uh oh!

Zafouche commented Oct 31, 2024

Uh oh!

AlexeiThornber commented Oct 31, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

Zafouche commented Oct 27, 2024 •

edited

Loading

Zafouche commented Oct 28, 2024 •

edited

Loading

Zafouche commented Oct 29, 2024 •

edited

Loading

Zafouche commented Oct 29, 2024 •

edited

Loading

Zafouche Oct 30, 2024 •

edited

Loading

Zafouche Oct 30, 2024 •

edited

Loading

Zafouche Oct 30, 2024 •

edited

Loading