-
Notifications
You must be signed in to change notification settings - Fork 8
Queue filter #82
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
Queue filter #82
Conversation
…mit.The UI adds a dialog for staging and unstaging edits from other users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few minor comments. Looks good code-wise; haven't actually tested the behavior but will do that before next release.
@@ -1652,42 +1662,143 @@ webui.ChangedFilesView = function(workspaceView, type, label) { | |||
} | |||
}; | |||
|
|||
self.getFileList = function(including, excluding) { | |||
var files = ""; | |||
function confirmActionForUnavailalbleFile(files, action) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: typo (unavailable)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed this.
Method OnBeforeDelete(InternalName As %String) As %Status | ||
{ | ||
set context = ##class(SourceControl.Git.PackageManagerContext).ForInternalName(InternalName) | ||
set InternalName = ##class(Utils).NormalizeInternalName(InternalName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: full classnames are preferred (though other code in the same neighborhood might not be great about that)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This happens in multiple places all throughout the project. It might be better to create a separate PR for finding and refactoring all these instances.
@@ -160,13 +160,17 @@ ClassMethod Uncommitted() As %SystemBase | |||
quit:key="" | |||
// Check that current user has files(key) uncommitted and only %Push if they do | |||
set filename = ##class(Utils).FullExternalName(key) | |||
set sc=##class(SourceControl.Git.Change).GetUncommitted(filename,.tAction,.tInternalName,.UncommittedUser,.tSource,.UncommittedLastUpdated) | |||
if $$$ISOK(sc) && ($data(tAction)&&(UncommittedUser=$username)) { | |||
if (($ISVALIDNUM(key)) && (files(key) '= "")){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to check ISVALIDNUM?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Utils.GitStatus()
, if we want to include all files, then for the files that don't have an internal name but are still tracked by git, I use $increment
to generate keys. For files that have internal names, the internal names are the keys and the filepath is the value associated with the key.
So in WebUIDriver.Uncommitted()
, if the key is a number and the value is not empty, then it indicates that the file is tracked by Git but not by the Uncommitted queue. Therefore, any user should be able to take action on those changes.
This PR closes #28