Feat/association manager #302
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…sociation screen if you have any Permission on it
…& create Association member @composable for Actions page
…odel to add a Role locally to reduce fetches from the database
… in UI + do it securely
…n to avoid future errors
|
Thanks again for your review Oskar ! I implemented everything you asked ; ) |
This was
linked to
issues
Dec 20, 2024
…rebase emulators data
armouldr
reviewed
Dec 20, 2024
Comment on lines
+119
to
+121
| # waiting for Arnaud's PR, temporarily commenting this | ||
| #- name: Run Node tests with Firestore and Storage emulators | ||
| # run: firebase emulators:exec --only firestore,storage 'npm run test' |
There was a problem hiding this comment.
Don't forget to uncomment this as soon as we have merged my PR :)
Comment on lines
+520
to
+533
| composeTestRule.waitUntil(10000) { | ||
| composeTestRule.onNodeWithTag(AssociationProfileTestTags.ACTIONS_PAGE).isDisplayed() | ||
| } | ||
| composeTestRule.onNodeWithTag(AssociationProfileTestTags.ACTIONS_PAGE).performClick() | ||
|
|
||
| composeTestRule.waitUntil(10000) { | ||
| composeTestRule | ||
| .onNodeWithTag(AssociationProfileActionsTestTags.ADD_EVENT_BUTTON) | ||
| .isDisplayed() | ||
| } | ||
|
|
||
| composeTestRule | ||
| .onNodeWithTag(AssociationProfileActionsTestTags.ADD_EVENT_BUTTON) | ||
| .assertIsDisplayed() |
There was a problem hiding this comment.
This is basic testing but it ensures that everything is well displayed.
| @@ -199,14 +199,11 @@ class AccountDetailsTest : TearDown() { | |||
| .onNodeWithTag(SocialsOverlayTestTags.SAVE_BUTTON) | |||
| .performScrollTo() | |||
| .performClick() | |||
| Thread.sleep(20000) | |||
There was a problem hiding this comment.
i think this was for test reasons, tell me if I'm wrong
Comment on lines
+172
to
+205
| fun testEventCardElementsExistEdit() { | ||
| setEventViewModel(listOf(sampleEvent)) | ||
| setEventScreen(sampleEvent, true) | ||
|
|
||
| Thread.sleep(10000) | ||
| composeTestRule | ||
| .onNodeWithTag(EventCardTestTags.EVENT_TITLE, useUnmergedTree = true) | ||
| .assertExists() | ||
| .assertTextEquals("Sample Event") | ||
|
|
||
| composeTestRule | ||
| .onNodeWithTag(EventCardTestTags.EVENT_MAIN_TYPE, useUnmergedTree = true) | ||
| .assertExists() | ||
| .assertTextEquals("Trip") | ||
|
|
||
| composeTestRule | ||
| .onNodeWithTag(EventCardTestTags.EVENT_LOCATION, useUnmergedTree = true) | ||
| .assertExists() | ||
| .assertTextEquals("Sample Location") | ||
|
|
||
| composeTestRule | ||
| .onNodeWithTag(EventCardTestTags.EVENT_DATE, useUnmergedTree = true) | ||
| .assertExists() | ||
| .assertTextEquals("20/07") | ||
|
|
||
| composeTestRule | ||
| .onNodeWithTag(EventCardTestTags.EVENT_TIME, useUnmergedTree = true) | ||
| .assertExists() | ||
| .assertTextEquals("00:00") | ||
|
|
||
| composeTestRule | ||
| .onNodeWithTag(EventCardTestTags.EVENT_CATCHY_DESCRIPTION, useUnmergedTree = true) | ||
| .assertExists() | ||
| .assertTextEquals("This is a catchy description.") |
There was a problem hiding this comment.
Great to add tests to some old code and not only to the new one ! 🚀
| @@ -193,6 +193,7 @@ class EventDetailsTest : TearDown() { | |||
| composeTestRule | |||
| .onNodeWithTag(EventDetailsTestTags.SHARE_BUTTON) | |||
| .assertDisplayComponentInScroll() | |||
| Thread.sleep(20000) | |||
There was a problem hiding this comment.
Same as above, this seems like debug instruction
| } | ||
|
|
||
| Row( | ||
| modifier = Modifier.fillMaxWidth().padding(vertical = 0.dp), |
| */ | ||
| @OptIn(ExperimentalMaterial3Api::class) | ||
| @Composable | ||
| fun <T> SearchBar( |
There was a problem hiding this comment.
This doesnt seem to be used anywhere :(
Comment on lines
+24
to
+31
| * | ||
| * @param T The type of the entity (e.g., Event, Member, Association). | ||
| * @param items The list of items to display in the pager. | ||
| * @param searchViewModel The view model handling the search logic. | ||
| * @param onItemSelected Callback when an item is selected from search results. | ||
| * @param cardContent A composable lambda that defines how to render each pager content. | ||
| * @param title The title displayed above the section. | ||
| * @param searchBar Composable function for rendering the search bar. |
| // Sliding Progress Bar (if more than one item exists) | ||
| if (items.size > 1) { | ||
| // Search Bar Composable | ||
| // Box(modifier = Modifier.fillMaxWidth().padding(horizontal = 20.dp)) { searchBar() } |
| Box( | ||
| modifier = | ||
| Modifier.fillMaxSize().drawBehind { | ||
| val totalWidth = sizeList.values.map { it.first }.sum() |
armouldr
approved these changes
Dec 20, 2024
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR aims to completely deploy the association permissions into the UI.
That means that the user is now able to :
Cool implementations :
When creating/editing a role, I thought a Color Picker is better than giving a HEX code or a Long. I found this superb Color Picker on github by SkyDoves - it's even compatible with Kotlin Multiplatform ! Good to know when we'll want to implement Swift.
Now Roles are only in the Association.roles and not copied in Association.members.role to avoid differences in the memory and better data optimization
Security measures :
Create and editing an association is now managed by cloud functions. That means that you don't directly perform the operations to the db - you just call a cloud function to check your permissions (and by that I mean the permissions of your roles) and proceed if you have at least "Add & Edit Association"
Cloud functions are now more safe : before, to know who's calling the function the userId was given as a string parameter. This is a dangerous security hole : as the uid of the users in the db is the same as the one visible in the Association dataclass, that means that everyone can know the uid of his comrade and thus give it the the cloud function instead of its own. Now, your userID is tokenized (only an authentificated user can have its own token) and then send to the cloud function which detokenize it to fall back on the userUid : ) If you think this is not the best way to proceed, maybe first read the documentation, it's right here - other possibilities are explained here.
Fixes :
Questions :
saveEventcloud function you might ask why I send the Firebase Timestamp as a string. It's because there's no way to pass it via a json Object which is the type used for the parameters of the cloud functions. So the solution is to format all the informations in a string in Kotlin and then rebuild the Timestamp in javascript.