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.

Sign up for GitHub

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

Add additional save protections to ScriptRunner #1072

Merged
merged 4 commits into from
Jan 24, 2024
Merged

Add additional save protections to ScriptRunner #1072

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0a3ed39
Add additional save protections to ScriptRunner
ryanmelt Jan 24, 2024
938d47b
Merge branch 'main' into scriptrunner_save_protections
ryanmelt Jan 24, 2024
ce67d72
Reorder scriptComplete and fix playwright
jmthomas Jan 24, 2024
fb5cba1
Unsubscribe from script events on complete. Make cable handle specia…
ryanmelt Jan 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
192 changes: 106 additions & 86 deletions .../plugins/packages/openc3-cosmos-tool-scriptrunner/src/tools/ScriptRunner/ScriptRunner.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -477,7 +477,7 @@ export default {
// },
},
filenameSelect: null,
currentFilename: null,
currentFilename: null, // This is the currently shown filename while running
showSave: false,
showAlert: false,
alertType: null,
Expand All @@ -499,6 +499,7 @@ export default {
files: {},
breakpoints: {},
filename: NEW_FILENAME,
saveAllowed: true,
tempFilename: null,
fileModified: '',
fileOpen: false,
Expand Down Expand Up @@ -577,9 +578,11 @@ export default {
}
},
computed: {
// This is the list of files shown in the select dropdown
fileList: function () {
// this.files is the list of all files seen while running
const filenames = Object.keys(this.files)
filenames.push(this.fullFilename)
filenames.push(this.fullFilename) // Make sure the currently shown filename is last
return [...new Set(filenames)] // ensure unique
},
environmentIcon: function () {
Expand All @@ -593,6 +596,7 @@ export default {
readOnly: function () {
return !!this.lockedBy
},
// Returns the currently shown filename
fullFilename: function () {
if (this.currentFilename) return this.currentFilename
// New filenames should not indicate modified
Expand All @@ -601,6 +605,7 @@ export default {
},
// It's annoying for people (and tests) to clear the <Untitled>
// when saving a new file so replace with blank
// This makes sure that string doesn't show up in the dialog
filenameOrBlank: function () {
return this.filename === NEW_FILENAME ? '' : this.filename
},
Expand Down Expand Up @@ -926,21 +931,21 @@ export default {
// Rebuild the command since that doesn't get stringified
this.recent = this.recent.map((item) => ({
...item,
command: (event) => {
command: async (event) => {
this.filename = event.label
this.reloadFile()
await this.reloadFile()
},
}))
}
if (this.$route.query?.file) {
this.filename = this.$route.query.file
this.reloadFile()
await this.reloadFile()
} else if (this.$route.params?.id) {
await this.tryLoadRunningScript(this.$route.params.id)
} else {
if (localStorage['script_runner__filename']) {
this.filename = localStorage['script_runner__filename']
this.reloadFile(false)
await this.reloadFile(false)
}
}
// TODO: Potentially still bad interactions with autoSave
Expand Down Expand Up @@ -1101,6 +1106,7 @@ export default {
}
},
// This only gets called when the user changes the filename dropdown
// Or when a user hits Go
fileNameChanged(filename) {
// Split off the '*' which indicates modified
filename = filename.split('*')[0]
Expand Down Expand Up @@ -1177,14 +1183,14 @@ export default {
.map((row) => row - range.start.row)
this.executeText(text, breakpoints)
},
executeText: function (text, breakpoints = []) {
async executeText(text, breakpoints = []) {
// Create a new temp script and open in new tab
const selectionTempFilename =
TEMP_FOLDER +
'/' +
format(Date.now(), 'yyyy_MM_dd_HH_mm_ss_SSS') +
'_temp.rb'
Api.post(`/script-api/scripts/${selectionTempFilename}`, {
await Api.post(`/script-api/scripts/${selectionTempFilename}`, {
data: {
text,
breakpoints,
Expand Down Expand Up @@ -1278,6 +1284,10 @@ export default {
}
},
async keydown(event) {
// Don't ever save if running or readonly
if (this.scriptId || this.editor.getReadOnly() === true) {
return
}
// NOTE: Chrome does not allow overriding Ctrl-N, Ctrl-Shift-N, Ctrl-T, Ctrl-Shift-T, Ctrl-W
// NOTE: metaKey == Command on Mac
if (
Expand Down Expand Up @@ -1352,7 +1362,7 @@ export default {
this.subscription = subscription
})
},
scriptComplete() {
async scriptComplete() {
this.disableSuiteButtons = false
this.startOrGoButton = START
this.pauseOrRetryButton = PAUSE
Expand All @@ -1368,9 +1378,10 @@ export default {
this.currentFilename = null
this.files = {} // Clear the file cache
// We may have changed the contents (if there were sub-scripts)
// so don't let the undo manager think this is a chanage
// so don't let the undo manager think this is a change
this.editor.session.getUndoManager().reset()
this.editor.setReadOnly(false)
await this.reloadFile() // Make sure the right file is shown
},
environmentHandler: function (event) {
this.scriptEnvironment.env = event
Expand All @@ -1384,6 +1395,7 @@ export default {
// like disabling start which could allow users to click start twice.
this.initScriptStart()
await this.saveFile('start')
this.saveAllowed = false
let filename = this.filename
if (this.filename === NEW_FILENAME) {
// NEW_FILENAME so use tempFilename created by saveFile()
Expand Down Expand Up @@ -1861,6 +1873,7 @@ export default {
this.tempFilename = null
this.files = {} // Clear the cached file list
this.editor.session.setValue('')
this.saveAllowed = true
this.fileModified = ''
this.suiteRunner = false
this.startOrGoDisabled = false
Expand Down Expand Up @@ -1971,9 +1984,9 @@ class TestSuite(Suite):
this.recent.unshift({
label: filename,
icon: fileIcon(filename),
command: (event) => {
command: async (event) => {
this.filename = event.label
this.reloadFile()
await this.reloadFile()
},
})
if (this.recent.length > 8) {
Expand All @@ -1995,8 +2008,9 @@ class TestSuite(Suite):
async reloadFile(showError = true) {
// Disable start while we're loading the file so we don't hit Start
// before it's fully loaded and then save over it with a blank file
this.saveAllowed = false
this.startOrGoDisabled = true
Api.get(`/script-api/scripts/${this.filename}`, {
await Api.get(`/script-api/scripts/${this.filename}`, {
headers: {
Accept: 'application/json',
'Ignore-Errors': '404',
Expand All @@ -2019,6 +2033,7 @@ class TestSuite(Suite):
const locked = response.data.locked
const breakpoints = response.data.breakpoints
this.setFile({ file, locked, breakpoints }, true)
this.saveAllowed = true
})
.catch((error) => {
if (showError === true) {
Expand Down Expand Up @@ -2124,86 +2139,91 @@ class TestSuite(Suite):
// saveFile takes a type to indicate if it was called by the Menu
// or automatically by 'Start' (to ensure a consistent backend file) or autoSave
async saveFile(type = 'menu') {
const breakpoints = this.getBreakpointRows()
if (this.filename === NEW_FILENAME) {
if (type === 'menu') {
// Menu driven saves on a new file should prompt SaveAs
this.saveAs()
return
} else {
if (this.tempFilename === null) {
let language = this.detectLanguage()
if (
language === 'ruby' ||
(type !== 'auto' && language == 'unknown')
) {
this.tempFilename =
TEMP_FOLDER +
'/' +
format(Date.now(), 'yyyy_MM_dd_HH_mm_ss_SSS') +
'_temp.rb'
} else if (language === 'python') {
this.tempFilename =
TEMP_FOLDER +
'/' +
format(Date.now(), 'yyyy_MM_dd_HH_mm_ss_SSS') +
'_temp.py'
} else {
// No autosave for unknown language
return
if (this.saveAllowed) {
const breakpoints = this.getBreakpointRows()
if (this.filename === NEW_FILENAME) {
if (type === 'menu') {
// Menu driven saves on a new file should prompt SaveAs
this.saveAs()
return
} else {
// start or auto with NEW_FILENAME
if (this.tempFilename === null) {
let language = this.detectLanguage()
if (
language === 'ruby' ||
(type !== 'auto' && language == 'unknown')
) {
this.tempFilename =
TEMP_FOLDER +
'/' +
format(Date.now(), 'yyyy_MM_dd_HH_mm_ss_SSS') +
'_temp.rb'
} else if (language === 'python') {
this.tempFilename =
TEMP_FOLDER +
'/' +
format(Date.now(), 'yyyy_MM_dd_HH_mm_ss_SSS') +
'_temp.py'
} else {
// No autosave for unknown language
return
}
this.filename = this.tempFilename
this.addToRecent(this.filename)
}
this.filename = this.tempFilename
this.addToRecent(this.filename)
}
}
}
this.showSave = true
await Api.post(`/script-api/scripts/${this.filename}`, {
data: {
text: this.editor.getValue(), // Pass in the raw file text
breakpoints,
},
})
.then((response) => {
if (response.status == 200) {
if (response.data.suites) {
this.startOrGoDisabled = true
this.suiteRunner = true
this.suiteMap = JSON.parse(response.data.suites)
this.showSave = true
await Api.post(`/script-api/scripts/${this.filename}`, {
data: {
text: this.editor.getValue(), // Pass in the raw file text
breakpoints,
},
})
.then((response) => {
if (response.status == 200) {
if (response.data.suites) {
this.startOrGoDisabled = true
this.suiteRunner = true
this.suiteMap = JSON.parse(response.data.suites)
} else {
this.startOrGoDisabled = false
this.suiteRunner = false
this.suiteMap = {}
}
if (response.data.error) {
this.suiteError = response.data.error
this.showSuiteError = true
}
this.fileModified = ''
setTimeout(() => {
this.showSave = false
}, 2000)
} else {
this.startOrGoDisabled = false
this.suiteRunner = false
this.suiteMap = {}
}
if (response.data.error) {
this.suiteError = response.data.error
this.showSuiteError = true
}
this.fileModified = ''
setTimeout(() => {
this.showSave = false
}, 2000)
} else {
this.alertType = 'error'
this.alertText = `Error saving file. Code: ${response.status} Text: ${response.statusText}`
this.showAlert = true
}
this.lockFile() // Ensure this file is locked for editing
})
.catch(({ response }) => {
this.showSave = false
this.alertType = 'error'
this.alertText = `Error saving file. Code: ${response.status} Text: ${response.statusText}`
// 422 error means we couldn't parse the script file into Suites
// response.data.suites holds the parse result
if (response.status == 422) {
this.alertType = 'error'
this.alertText = response.data.suites
} else {
this.alertType = 'error'
this.alertText = `Error saving file. Code: ${response.status} Text: ${response.statusText}`
}
this.showAlert = true
}
this.lockFile() // Ensure this file is locked for editing
})
.catch(({ response }) => {
this.showSave = false
// 422 error means we couldn't parse the script file into Suites
// response.data.suites holds the parse result
if (response.status == 422) {
this.alertType = 'error'
this.alertText = response.data.suites
} else {
this.alertType = 'error'
this.alertText = `Error saving file. Code: ${response.status} Text: ${response.statusText}`
}
this.showAlert = true
})
})
} else {
this.setError('Attempt to save file when not allowed')
}
},
saveAs() {
this.showSaveAs = true
Expand Down
Loading