Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Recent Section #321

Merged
merged 16 commits into from
Jan 27, 2025
Merged

Conversation

eylulnc
Copy link
Contributor

@eylulnc eylulnc commented Jan 20, 2025

Problem Description

The Recent section was missing in the conversation list, and no dedicated feature existed to display conversations based on their temporal relevance. This created inconsistencies with the iOS and Web implementations, which already have a Recent section for filtering conversations within a defined time range (e.g., for exercises, lectures, and exams).

Changes

New Recent Section Implementation:

  • Added a dedicated Recent section to the conversation list to align with the iOS and Web platforms.
  • The section filters conversations based on the following logic:
    • Exercises: Include conversations with release or due dates within 10 days before or after the current date.
    • Lectures: Include conversations with start or end dates within 10 days before or after the current date.
      - Exams: Include conversations created within 10 days before or after the current date.
  • Conversations outside these ranges are excluded from the Recent section.

Steps for testing

  • Create exercises, exams, and lectures in the system with varying dates:
    • Exercises with release or due dates within 10 days before or after the current date.
    • Lectures with start or end dates within 10 days before or after the current date.
      - Exams with creation dates within 10 days before or after the current date.
    • Additionally, create conversations outside the 10-day range and mark some conversations as archived or hidden.
  • Navigate to Conversation Overview:
  • Verify Recent Section Logic:
    • Exercises:
      • Check that exercises with release or due dates within the 10-day range appear in the Recent section.
      • Verify that exercises outside the date range do not appear.
    • Lectures:
      • Ensure lectures with start or end dates within the 10-day range appear in the Recent section.
      • Confirm that lectures outside the date range are not listed.
        Exams:
        Verify that exams with creation dates within the 10-day range are displayed.
        Check that exams outside the range are not included.
    • Exclude Archived/Hidden Conversations:
      • Verify that conversations marked as archived or hidden are excluded from the Recent section, even if their dates fall within the 10-day range.

Screenshots

Screenshot 2025-01-20 at 17 23 12

@eylulnc eylulnc self-assigned this Jan 20, 2025
@eylulnc eylulnc added the ready for review This PR can be reviewed label Jan 20, 2025
Copy link
Collaborator

@FelberMartin FelberMartin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with exam, lecture, and exercise, works as described.

While testing, I noticed that we can currently not straight-away tell which category (exam, lecture, exercise) the chat in the recents section belong to. We should not remove the conversation-prefix here imo

@eylulnc
Copy link
Contributor Author

eylulnc commented Jan 25, 2025

Tested with exam, lecture, and exercise, works as described.

While testing, I noticed that we can currently not straight-away tell which category (exam, lecture, exercise) the chat in the recents section belong to. We should not remove the conversation-prefix here imo

You are right, I have updated the prefix

…into feature/communication/recent-section

# Conflicts:
#	feature/course-view/src/debug/kotlin/de/tum/informatics/www1/artemis/native_app/feature/courseview/MessagingScreenshots.kt
#	feature/metis/manage-conversations/src/main/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metis/manageconversations/service/storage/ConversationPreferenceService.kt
#	feature/metis/manage-conversations/src/main/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metis/manageconversations/service/storage/impl/ConversationPreferenceStorageServiceImpl.kt
#	feature/metis/manage-conversations/src/main/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metis/manageconversations/ui/conversation/overview/ConversationList.kt
#	feature/metis/manage-conversations/src/main/kotlin/de/tum/informatics/www1/artemis/native_app/feature/metis/manageconversations/ui/conversation/overview/ConversationOverviewViewModel.kt
#	feature/metis/manage-conversations/src/main/res/values/conversation_overview_strings.xml
@eylulnc eylulnc requested a review from FelberMartin January 25, 2025 10:00
@eylulnc
Copy link
Contributor Author

eylulnc commented Jan 25, 2025

@FelberMartin @julian-wls should I remove the exam part as we speak and work it on after we decide to add the exam module. Or we I leave it as it is?

Copy link
Collaborator

@FelberMartin FelberMartin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm now

@FelberMartin
Copy link
Collaborator

@FelberMartin @julian-wls should I remove the exam part as we speak and work it on after we decide to add the exam module. Or we I leave it as it is?

For me it would be fine to leave it as is for now, but do however you see fit, I dont have a strong opinion on it :D

@eylulnc
Copy link
Contributor Author

eylulnc commented Jan 26, 2025

@FelberMartin @julian-wls should I remove the exam part as we speak and work it on after we decide to add the exam module. Or we I leave it as it is?

For me it would be fine to leave it as is for now, but do however you see fit, I dont have a strong opinion on it :D

Just to align with iOS I will remove it and in the future we can add it in a more meaningful way

Copy link
Contributor

@julian-wls julian-wls left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just tested it. Works as expected.
Code lgtm.

@FelberMartin FelberMartin added ready to merge This PR can be merged and removed ready for review This PR can be reviewed labels Jan 27, 2025
@FelberMartin FelberMartin merged commit 5e94e18 into develop Jan 27, 2025
5 checks passed
@FelberMartin FelberMartin deleted the feature/communication/recent-section branch January 27, 2025 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge This PR can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants