Skip to content

Commit

Permalink
Code review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
jaakkonakaza committed Aug 23, 2024
1 parent d1598d0 commit ee5960f
Show file tree
Hide file tree
Showing 34 changed files with 110 additions and 361 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,19 +35,6 @@ class OpenExerciseItemAction : AnAction() {
}

private fun browse(url: String, project: Project) {
/* Embedded browser for the future
val fileEditorManager = FileEditorManager.getInstance(project)
fileEditorManager.allEditors.forEach { editor: FileEditor ->
if (editor.file != null && editor.file.extension == "aplus") {
(editor as BrowserEditor).loadURL(url)
return
}
}
val file = LightVirtualFile("browse.aplus", "url:$url")
application.invokeLater {
fileEditorManager.openFile(file, true)
}
*/
BrowserUtil.browse(url, project)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import com.intellij.openapi.actionSystem.ActionUpdateThread
import com.intellij.openapi.actionSystem.AnAction
import com.intellij.openapi.actionSystem.AnActionEvent
import com.intellij.openapi.components.service
import fi.aalto.cs.apluscourses.services.course.CourseManager
import fi.aalto.cs.apluscourses.services.exercise.SelectedExercise
import fi.aalto.cs.apluscourses.services.exercise.ShowFeedback
import fi.aalto.cs.apluscourses.ui.exercise.ExercisesView.SubmissionResultItem
Expand All @@ -18,8 +19,14 @@ class ShowFeedbackAction : AnAction() {
}

override fun update(e: AnActionEvent) {
val feedbackCss = e.project?.service<CourseManager>()?.state?.feedbackCss
if (feedbackCss == null || feedbackCss.isEmpty()) {
e.presentation.isVisible = false
return
}
val selected = e.project?.service<SelectedExercise>()?.selectedExerciseTreeItem
e.presentation.isEnabled = selected != null && selected is SubmissionResultItem
e.presentation.isEnabled =
selected != null && selected is SubmissionResultItem && selected.exercise.isSubmittable
}

override fun getActionUpdateThread(): ActionUpdateThread = ActionUpdateThread.EDT
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import fi.aalto.cs.apluscourses.MyBundle
import fi.aalto.cs.apluscourses.api.CourseConfig
import fi.aalto.cs.apluscourses.api.CourseConfig.resourceUrls
import fi.aalto.cs.apluscourses.notifications.CourseVersionOutdatedError
import fi.aalto.cs.apluscourses.notifications.CourseVersionOutdatedWarning
import fi.aalto.cs.apluscourses.notifications.CourseVersionTooNewError
import fi.aalto.cs.apluscourses.services.CoursesClient
import fi.aalto.cs.apluscourses.services.Notifier
Expand Down Expand Up @@ -77,15 +78,22 @@ internal class InitializationActivity() :
val requiredVersion = courseConfig.version
logger.info("Starting initialization for course ${courseConfig.name} with required plugin version $requiredVersion and installed version $pluginVersion")

val versionComparison = requiredVersion.comparisonStatus(pluginVersion)
val versionComparison = pluginVersion.comparisonStatus(requiredVersion)

when (versionComparison) {
ComparisonStatus.MAJOR_TOO_OLD -> {
logger.warn("A+ Courses version outdated: installed $pluginVersion, required $requiredVersion")
Notifier.notify(CourseVersionOutdatedError(), project)
InitializationStatus.setIsIoError(project)
CourseManager.getInstance(project).fireNetworkError()
return
}

ComparisonStatus.MINOR_TOO_OLD -> {
logger.warn("A+ Courses version outdated: installed $pluginVersion, required $requiredVersion")
Notifier.notify(CourseVersionOutdatedWarning(), project)
}

ComparisonStatus.MAJOR_TOO_NEW -> {
logger.warn("A+ Courses version too new: installed $pluginVersion, required $requiredVersion")
Notifier.notify(CourseVersionTooNewError(), project)
Expand Down
10 changes: 7 additions & 3 deletions src/main/kotlin/fi/aalto/cs/apluscourses/api/CourseConfig.kt
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package fi.aalto.cs.apluscourses.api

import com.intellij.openapi.project.Project
import fi.aalto.cs.apluscourses.notifications.CourseConfigurationError
import fi.aalto.cs.apluscourses.services.CoursesClient
import fi.aalto.cs.apluscourses.services.Notifier
import fi.aalto.cs.apluscourses.services.course.CourseFileManager
import fi.aalto.cs.apluscourses.utils.Version
import io.ktor.client.statement.bodyAsText
Expand Down Expand Up @@ -138,7 +140,7 @@ object CourseConfig {
val name: String,
val aPlusUrl: String,
val languages: List<String>,
val version: Version,
val version: Version = Version.DEFAULT,
val resources: Resources? = null,
val requiredPlugins: List<RequiredPlugin> = emptyList(),
val vmOptions: Map<String, String> = emptyMap(),
Expand Down Expand Up @@ -245,10 +247,12 @@ object CourseConfig {
val courseConfig = CoursesClient.getInstance(project).get(url).bodyAsText()
return deserialize(courseConfig)
} catch (e: Exception) {
println(e)
when (e) {
is IOException, is UnresolvedAddressException -> throw e
else -> return null
else -> {
Notifier.notify(CourseConfigurationError(e), project)
return null
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,14 +226,16 @@ internal class APlusModuleBuilder : ModuleBuilder() {
checkBox("Leave IntelliJ settings unchanged").bindSelected(dontImportSettings)
}
}
group("Required plugins") {
row {
text("The following plugins required by the course will be installed.")
}
row {
placeholder().apply {
placeholder = this
}.resizableColumn().align(AlignX.FILL)
if (courseConfig.requiredPlugins.isNotEmpty()) {
group("Required plugins") {
row {
text("The following plugins required by the course will be installed.")
}
row {
placeholder().apply {
placeholder = this
}.resizableColumn().align(AlignX.FILL)
}
}
}
if (this@APlusModuleBuilder.programmingLanguage == "scala") {
Expand All @@ -254,6 +256,7 @@ internal class APlusModuleBuilder : ModuleBuilder() {

component.revalidate()
component.repaint()
if (courseConfig.requiredPlugins.isEmpty()) return
application.service<Plugins>().runInBackground(courseConfig.requiredPlugins) { components ->
placeholder?.component = panel {
components.map {
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ import com.intellij.notification.NotificationType
import fi.aalto.cs.apluscourses.MyBundle
import fi.aalto.cs.apluscourses.services.PluginSettings

class CourseConfigurationError
/**
* Constructs a notification that notifies the user of an error that occurred while attempting to
* A notification that notifies the user of an error that occurred while attempting to
* parse a course configuration file.
*
* @param exception An exception that caused this notification.
*/(val exception: Exception) : Notification(
*/
class CourseConfigurationError(val exception: Exception) : Notification(
PluginSettings.A_PLUS,
MyBundle.message("notification.CourseConfigurationError.title"),
MyBundle.message(
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@ import com.intellij.notification.NotificationType
import fi.aalto.cs.apluscourses.MyBundle
import fi.aalto.cs.apluscourses.services.PluginSettings

class CourseVersionOutdatedError
/**
* Notification to be shown when the plugin is outdated with respect to the current course.
* A notification to be shown when the plugin is outdated with respect to the current course.
* This represents a mismatch in the major version and is an error.
*/
: Notification(
class CourseVersionOutdatedError : Notification(
PluginSettings.A_PLUS, MyBundle.message("notification.CourseVersionError.title"),
MyBundle.message("notification.CourseVersionOutdatedError.content"), NotificationType.ERROR
)
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@ import com.intellij.notification.NotificationType
import fi.aalto.cs.apluscourses.MyBundle
import fi.aalto.cs.apluscourses.services.PluginSettings

class CourseVersionOutdatedWarning
/**
* Notification to be shown when the plugin is outdated with respect to the current course.
* A notification to be shown when the plugin is outdated with respect to the current course.
* This represents a mismatch in the minor version and is a warning.
*/
: Notification(
class CourseVersionOutdatedWarning : Notification(
PluginSettings.A_PLUS, MyBundle.message("notification.CourseVersionError.title"),
MyBundle.message("notification.CourseVersionOutdatedWarning.content"), NotificationType.WARNING
)
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@ import com.intellij.notification.NotificationType
import fi.aalto.cs.apluscourses.MyBundle
import fi.aalto.cs.apluscourses.services.PluginSettings

class CourseVersionTooNewError
/**
* Notification to be shown when the plugin is too new to support the current course.
* A notification to be shown when the plugin is too new to support the current course.
*/
: Notification(
class CourseVersionTooNewError : Notification(
PluginSettings.A_PLUS, MyBundle.message("notification.CourseVersionError.title"),
MyBundle.message("notification.CourseVersionTooNewError.content"), NotificationType.ERROR
)

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,11 @@ import fi.aalto.cs.apluscourses.services.PluginSettings
import fi.aalto.cs.apluscourses.services.course.CourseManager
import fi.aalto.cs.apluscourses.utils.SubmissionResultUtil

//import fi.aalto.cs.apluscourses.services.PluginSettings.Companion.getInstance

/**
* A notification that notifies the user that feedback is available for a submission.
* The notification contains a link that can be used to open the feedback and the amount of
* points the submission got.
*/
class FeedbackAvailableNotification(
submissionResult: SubmissionResult,
exercise: Exercise,
Expand All @@ -27,15 +30,10 @@ class FeedbackAvailableNotification(
),
NotificationType.INFORMATION
) {
/**
* Construct a notification that notifies the user that feedback is available for a submission.
* The notification contains a link that can be used to open the feedback and the amount of
* points the submission got.
*/
init {
if (CourseManager.getInstance(project).state.feedbackCss != null) {
super.addAction(ShowFeedbackNotificationAction(submissionResult, exercise))
addAction(ShowFeedbackNotificationAction(submissionResult, exercise))
}
super.addAction(OpenSubmissionNotificationAction(submissionResult))
addAction(OpenSubmissionNotificationAction(submissionResult))
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,16 @@ import com.intellij.notification.NotificationType
import fi.aalto.cs.apluscourses.MyBundle
import fi.aalto.cs.apluscourses.services.PluginSettings

class MissingDependencyNotification
/**
* Construct a missing module notification which explains that modules with the given names couldn't be found.
* A missing module notification which explains that modules with the given names couldn't be found.
*/
(moduleNames: Set<String?>) : Notification(
class MissingDependencyNotification(moduleNames: Set<String>) : Notification(
PluginSettings.A_PLUS,
MyBundle.message("notification.MissingDependencyNotification.title"),
MyBundle.message(
"notification.MissingDependencyNotification.content",
java.lang.String.join(", ", moduleNames)
moduleNames.joinToString(", ")
),
NotificationType.ERROR
)
// TODO show
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,12 @@ import com.intellij.notification.Notification
import com.intellij.notification.NotificationType
import fi.aalto.cs.apluscourses.MyBundle
import fi.aalto.cs.apluscourses.services.PluginSettings
import java.nio.file.Path

/**
* Construct a missing file notification that explains that a file with the given name couldn't be
* A missing file notification that explains that a file with the given name couldn't be
* found in the given module.
*/
class MissingFileNotification
/**
* Construct a missing file notification that explains that a file with the given name couldn't be
* found in the given module.
*/ @JvmOverloads constructor(val path: Path, val filename: String, download: Boolean = false) : Notification(
class MissingFileNotification(path: String, filename: String, download: Boolean = false) : Notification(
PluginSettings.A_PLUS,
MyBundle.message("notification.MissingFileNotification.title"),
MyBundle.message(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,12 @@ import com.intellij.notification.NotificationType
import fi.aalto.cs.apluscourses.MyBundle
import fi.aalto.cs.apluscourses.services.PluginSettings

class MissingModuleNotification
/**
* Construct a missing module notification that explains that a module with the given name
* couldn't be found.
*/(val moduleName: String) : Notification(
* Construct a missing module notification that explains that a module couldn't be found.
*/
class MissingModuleNotification : Notification(
PluginSettings.A_PLUS,
MyBundle.message("notification.MissingModuleNotification.title"),
MyBundle.message("notification.MissingModuleNotification.content", moduleName),
MyBundle.message("notification.MissingModuleNotification.content"),
NotificationType.ERROR
)
Loading

0 comments on commit ee5960f

Please sign in to comment.