Skip to content

[REVIEW] - Review Updated Class Diagrams #77

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

Closed
basak-tepe opened this issue Mar 19, 2025 · 9 comments
Closed

[REVIEW] - Review Updated Class Diagrams #77

basak-tepe opened this issue Mar 19, 2025 · 9 comments
Assignees
Labels
documentation Improvements or additions to documentation review only for the review task itself

Comments

@basak-tepe
Copy link
Contributor

📝 Review Summary

I have updated the Class Diagram.

🔄 Changes Made

Modifications are listed here #75
In addition, I added missing functions from SRS and a WeatherService class to include Weather API data.

🔗 Related Topic

No response

⚠️ Concerns

No response

@basak-tepe basak-tepe added documentation Improvements or additions to documentation enhancement New feature or request labels Mar 19, 2025
@bahadirkuscan
Copy link
Contributor

bahadirkuscan commented Mar 19, 2025

@basak-tepe First issue that grabs my attention upon review, why Garden contains ForumPost? Don't we have 2 completely seperated modules in the platform one being the forum and one for gardens?

@basak-tepe
Copy link
Contributor Author

As we discussed and fixed in #74, the forum and garden spaces are separate. It is error propagation from requirements & should be fixed. Thank you. Any other feedback?

@bahadirkuscan
Copy link
Contributor

I'm working on a detailed review but I'm quite busy these days, I will report the results as soon as possible.

@basak-tepe basak-tepe added review only for the review task itself and removed enhancement New feature or request labels Mar 22, 2025
@bahadirkuscan
Copy link
Contributor

Here is the list of things I think are missing (related to integrity):

  • Garden types (Refer to Customer Meeting, this is also missing in the SRS)
  • Users self-assigning tasks (2.6)
  • Calendar Structure (3.4)
  • Nearby garden recommendation (1.7)
  • Enable/disable notifications (6.2)
  • Moderators alter posts (7.1.3.2)
  • Moderators remove gardens (7.1.3.4)
  • Task summaries (7.1.5.3)
  • Managers modify tasks (7.6.1.5)
  • Workers update task status (7.6.2.3)

Corresponding recommended solutions:

  • Add "type" field to "Garden", update the SRS
  • Add "selfAssignTask()" function to "Worker"
  • Add new class "Calendar" that has associations with "Garden" and "Task"
  • Add "LocationService" class, add "recommendGarden()" function to "Member"
  • Add "enable()" and "disable()" functions to "Notification"
  • Add "alterPost()" function to "Moderator"
  • Add "removeGarden()" function to "Moderator"
  • Add "summary" field to "Task"
  • Add "modifyTask()" function to "Manager"
  • Add "updateTaskStatus()" function to "Worker"

@basak-tepe waiting for your opinions on this, I can deploy the changes if we agree

@basak-tepe
Copy link
Contributor Author

basak-tepe commented Mar 25, 2025

Thank you for the feedback!

I agree with you on:

  • Add "type" field to "Garden", update the SRS : yes we should. Example garden types can be greenhouse, open-air, agricultural etc.
  • Add "selfAssignTask()" function to "Worker" : yes
  • Add "updateTaskStatus()" function to "Worker" yes
  • Add "LocationService" class, add "recommendGarden()" function to "Member" : yes. But we should note that is should be a lower priority in development phase.
  • _Add "enable()" and "disable()" functions to "Notification": _ yes

Addition to your findings from my scenario reviews & srs cross-check

  • Notification class needs a setReminder() function, see scenario 1.
  • Notification class needs a channel field, for in-app or email notifications. See SRS 6.1

Things we don't need

  • Add new class "Calendar" that has associations with "Garden" and "Task" : no, not needed!. We don't need a calendar object. Garden and Task have their date fields. To be able to render a calendar on UI, this is enough.
  • Add "removeGarden()" function to "Moderator" : no. The garden class already has an delete function. To this function, we should add a permission check.
  • Add "summary" field to "Task": no. The description and task name is enough. A task summary should cannot be any different than a task description.
  • Add "modifyTask()" function to "Manager": no. The task class already has an update function. To this function, we should add a permission check.
  • Add "alterPost()" function to "Moderator" no. The forumpost class already has an edit function. To this function, we should add a permission check.

@basak-tepe
Copy link
Contributor Author

We also need to incorporate the following feedback on moodle

  • Method formats should specify return types (e.g., 'getID(): String')
  • Function return types are missing
  • GardenRole is linked but TaskStatus is not
  • Many-to-many relationships needed
  • Admin functions are missing

@SuleymanTolgaAcar
Copy link
Contributor

Some methods used in the sequence diagrams are missing in the class diagram. For example, addComment() method in the sequence diagram 3. We need to add the needed methods to the class diagram as well.

@basak-tepe
Copy link
Contributor Author

I will be employing the changes.

@basak-tepe
Copy link
Contributor Author

All updates are completed, closing the issue. Thanks everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation review only for the review task itself
Projects
None yet
Development

No branches or pull requests

3 participants