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

Remove thesis track and fix imports #694

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

niclasheun
Copy link
Collaborator

Description

This is a cleanup PR, which removes the Thesis Track completely from this project, as ThesisTrack has its own repo and every thesis related URL request is already redirected.

Type of change

  • Bug fix (updating dependencies)
  • I updated a version of jgit in the gradle config file, so that it starts up without issues
  • Cleanup
  • Removed all Thesis Track related components and controllers on client and server side
  • wrote a database migration to delete the thesis track related tables from the database

How Has This Been Tested?

I tested the database migration on my local machine. Further, I made sure to only remove dead code, and to really remove everything which is solely Thesis Track related

CAREFUL: This PR includes a database migration, which will delete all Thesis related database tables from the prompt database. Please review first, if this database migration is wanted/correct and if the data in these database tables is not needed any more.

@Mtze
Copy link
Member

Mtze commented Sep 30, 2024

Thanks for the change - I'd like to postpone the merge of this until after the intro course since this is a pretty hefty change :)

Copy link
Member

@bgeisb bgeisb left a comment

Choose a reason for hiding this comment

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

I have one small change request regarding the permissions needed to access the mailing management console. The rest looks good to me.

Copy link
Member

Choose a reason for hiding this comment

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

The route /management/mailing should now only have permission={[Permission.PM]}.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

in the route /management/mailing is an array of who as access and it is an 'OR' array. So persons with either PM or Chair_Member have access, which is fine in my opinion.

I think what you mean are the access rights to the mail templates, which are set in the server.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants