Skip to content

Drop team-specific logic from the CLI #1206

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
24 changes: 7 additions & 17 deletions cmd/download.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ type download struct {
token, apibaseurl, workspace string

// optional
track, team string
track string
forceoverwrite bool

payload *downloadPayload
Expand All @@ -156,7 +156,6 @@ func newDownload(flags *pflag.FlagSet, usrCfg *viper.Viper) (*download, error) {
if err != nil {
return nil, err
}
d.team, err = flags.GetString("team")
if err != nil {
return nil, err
}
Expand All @@ -176,7 +175,7 @@ func newDownload(flags *pflag.FlagSet, usrCfg *viper.Viper) (*download, error) {
if err = d.needsUserConfigValues(); err != nil {
return nil, err
}
if err = d.needsSlugWhenGivenTrackOrTeam(); err != nil {
if err = d.needsSlugWhenGivenTrack(); err != nil {
return nil, err
}

Expand Down Expand Up @@ -226,9 +225,6 @@ func (d download) buildQueryParams(url *netURL.URL) {
if d.track != "" {
query.Add("track_id", d.track)
}
if d.team != "" {
query.Add("team_id", d.team)
}
}
url.RawQuery = query.Encode()
}
Expand Down Expand Up @@ -256,11 +252,11 @@ func (d download) needsUserConfigValues() error {
return nil
}

// needsSlugWhenGivenTrackOrTeam ensures that track/team arguments are also given with a slug.
// (track/team meaningless when given a uuid).
func (d download) needsSlugWhenGivenTrackOrTeam() error {
if (d.team != "" || d.track != "") && d.slug == "" {
return errors.New("--track or --team requires --exercise (not --uuid)")
// needsSlugWhenGivenTrack ensures that track arguments are also given with a slug.
// (track meaningless when given a uuid).
func (d download) needsSlugWhenGivenTrack() error {
if d.track != "" && d.slug == "" {
return errors.New("--track or requires --exercise (not --uuid)")
}
return nil
}
Expand All @@ -269,10 +265,6 @@ type downloadPayload struct {
Solution struct {
ID string `json:"id"`
URL string `json:"url"`
Team struct {
Name string `json:"name"`
Slug string `json:"slug"`
} `json:"team"`
User struct {
Handle string `json:"handle"`
IsRequester bool `json:"is_requester"`
Expand Down Expand Up @@ -303,7 +295,6 @@ func (dp downloadPayload) metadata() workspace.ExerciseMetadata {
return workspace.ExerciseMetadata{
AutoApprove: dp.Solution.Exercise.AutoApprove,
Track: dp.Solution.Exercise.Track.ID,
Team: dp.Solution.Team.Slug,
ExerciseSlug: dp.Solution.Exercise.ID,
ID: dp.Solution.ID,
URL: dp.Solution.URL,
Expand Down Expand Up @@ -361,7 +352,6 @@ func setupDownloadFlags(flags *pflag.FlagSet) {
flags.StringP("uuid", "u", "", "the solution UUID")
flags.StringP("track", "t", "", "the track ID")
flags.StringP("exercise", "e", "", "the exercise slug")
flags.StringP("team", "T", "", "the team slug")
flags.BoolP("force", "F", false, "overwrite existing exercise directory")
}

Expand Down
30 changes: 6 additions & 24 deletions cmd/download_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,19 +160,14 @@ func TestDownload(t *testing.T) {
expectedDir: filepath.Join("users", "alice"),
flags: map[string]string{"uuid": "bogus-id"},
},
{
requester: true,
expectedDir: filepath.Join("teams", "bogus-team"),
flags: map[string]string{"exercise": "bogus-exercise", "track": "bogus-track", "team": "bogus-team"},
},
}

for _, tc := range testCases {
tmpDir, err := os.MkdirTemp("", "download-cmd")
defer os.RemoveAll(tmpDir)
assert.NoError(t, err)

ts := fakeDownloadServer(strconv.FormatBool(tc.requester), tc.flags["team"])
ts := fakeDownloadServer(strconv.FormatBool(tc.requester))
defer ts.Close()

v := viper.New()
Expand Down Expand Up @@ -221,10 +216,6 @@ func TestDownloadToExistingDirectory(t *testing.T) {
exerciseDir: filepath.Join("bogus-track", "bogus-exercise"),
flags: map[string]string{"exercise": "bogus-exercise", "track": "bogus-track"},
},
{
exerciseDir: filepath.Join("teams", "bogus-team", "bogus-track", "bogus-exercise"),
flags: map[string]string{"exercise": "bogus-exercise", "track": "bogus-track", "team": "bogus-team"},
},
}

for _, tc := range testCases {
Expand All @@ -235,7 +226,7 @@ func TestDownloadToExistingDirectory(t *testing.T) {
err = os.MkdirAll(filepath.Join(tmpDir, tc.exerciseDir), os.FileMode(0755))
assert.NoError(t, err)

ts := fakeDownloadServer("true", "")
ts := fakeDownloadServer("true")
defer ts.Close()

v := viper.New()
Expand Down Expand Up @@ -273,10 +264,6 @@ func TestDownloadToExistingDirectoryWithForce(t *testing.T) {
exerciseDir: filepath.Join("bogus-track", "bogus-exercise"),
flags: map[string]string{"exercise": "bogus-exercise", "track": "bogus-track"},
},
{
exerciseDir: filepath.Join("teams", "bogus-team", "bogus-track", "bogus-exercise"),
flags: map[string]string{"exercise": "bogus-exercise", "track": "bogus-track", "team": "bogus-team"},
},
}

for _, tc := range testCases {
Expand All @@ -287,7 +274,7 @@ func TestDownloadToExistingDirectoryWithForce(t *testing.T) {
err = os.MkdirAll(filepath.Join(tmpDir, tc.exerciseDir), os.FileMode(0755))
assert.NoError(t, err)

ts := fakeDownloadServer("true", "")
ts := fakeDownloadServer("true")
defer ts.Close()

v := viper.New()
Expand All @@ -310,7 +297,7 @@ func TestDownloadToExistingDirectoryWithForce(t *testing.T) {
}
}

func fakeDownloadServer(requestor, teamSlug string) *httptest.Server {
func fakeDownloadServer(requestor string) *httptest.Server {
mux := http.NewServeMux()
server := httptest.NewServer(mux)

Expand All @@ -327,15 +314,11 @@ func fakeDownloadServer(requestor, teamSlug string) *httptest.Server {
})

mux.HandleFunc("/solutions/latest", func(w http.ResponseWriter, r *http.Request) {
team := "null"
if teamSlug := r.FormValue("team_id"); teamSlug != "" {
team = fmt.Sprintf(`{"name": "Bogus Team", "slug": "%s"}`, teamSlug)
}
payloadBody := fmt.Sprintf(payloadTemplate, requestor, team, server.URL+"/")
payloadBody := fmt.Sprintf(payloadTemplate, requestor, server.URL+"/")
fmt.Fprint(w, payloadBody)
})
mux.HandleFunc("/solutions/bogus-id", func(w http.ResponseWriter, r *http.Request) {
payloadBody := fmt.Sprintf(payloadTemplate, requestor, "null", server.URL+"/")
payloadBody := fmt.Sprintf(payloadTemplate, requestor, server.URL+"/")
fmt.Fprint(w, payloadBody)
})

Expand Down Expand Up @@ -415,7 +398,6 @@ const payloadTemplate = `
"handle": "alice",
"is_requester": %s
},
"team": %s,
"exercise": {
"id": "bogus-exercise",
"instructions_url": "http://example.com/bogus-exercise",
Expand Down
2 changes: 1 addition & 1 deletion cmd/submit.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ func (s *submitCmdContext) printResult(metadata *workspace.ExerciseMetadata) {
%s
`
suffix := "View it at:\n\n "
if metadata.AutoApprove && metadata.Team == "" {
if metadata.AutoApprove {
suffix = "You can complete the exercise and unlock the next core exercise at:\n"
}
fmt.Fprintf(Err, msg, suffix)
Expand Down
47 changes: 0 additions & 47 deletions cmd/submit_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -425,53 +425,6 @@ func TestSubmitWithEnormousFile(t *testing.T) {
}
}

func TestSubmitFilesForTeamExercise(t *testing.T) {
co := newCapturedOutput()
co.override()
defer co.reset()

// The fake endpoint will populate this when it receives the call from the command.
submittedFiles := map[string]string{}
ts := fakeSubmitServer(t, submittedFiles)
defer ts.Close()

tmpDir, err := os.MkdirTemp("", "submit-files")
assert.NoError(t, err)

dir := filepath.Join(tmpDir, "teams", "bogus-team", "bogus-track", "bogus-exercise")
os.MkdirAll(filepath.Join(dir, "subdir"), os.FileMode(0755))
writeFakeMetadata(t, dir, "bogus-track", "bogus-exercise")

file1 := filepath.Join(dir, "file-1.txt")
err = os.WriteFile(file1, []byte("This is file 1."), os.FileMode(0755))
assert.NoError(t, err)

file2 := filepath.Join(dir, "subdir", "file-2.txt")
err = os.WriteFile(file2, []byte("This is file 2."), os.FileMode(0755))
assert.NoError(t, err)

v := viper.New()
v.Set("token", "abc123")
v.Set("workspace", tmpDir)
v.Set("apibaseurl", ts.URL)

cfg := config.Config{
Dir: tmpDir,
UserViperConfig: v,
}

files := []string{
file1, file2,
}
err = runSubmit(cfg, pflag.NewFlagSet("fake", pflag.PanicOnError), files)
assert.NoError(t, err)

assert.Equal(t, 2, len(submittedFiles))

assert.Equal(t, "This is file 1.", submittedFiles["file-1.txt"])
assert.Equal(t, "This is file 2.", submittedFiles["subdir/file-2.txt"])
}

func TestSubmitOnlyEmptyFile(t *testing.T) {
co := newCapturedOutput()
co.override()
Expand Down
1 change: 0 additions & 1 deletion shell/exercism.fish
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ complete -f -c exercism -n "__fish_seen_subcommand_from configure" -s s -l show
complete -f -c exercism -n "__fish_use_subcommand" -a "download" -d "Downloads and saves a specified submission into the local system"
complete -f -c exercism -n "__fish_seen_subcommand_from download" -s e -l exercise -d "the exercise slug"
complete -f -c exercism -n "__fish_seen_subcommand_from download" -s h -l help -d "help for download"
complete -f -c exercism -n "__fish_seen_subcommand_from download" -s T -l team -d "the team slug"
complete -f -c exercism -n "__fish_seen_subcommand_from download" -s t -l track -d "the track ID"
complete -f -c exercism -n "__fish_seen_subcommand_from download" -s u -l uuid -d "the solution UUID"

Expand Down
4 changes: 0 additions & 4 deletions workspace/exercise_metadata.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ type ExerciseMetadata struct {
Track string `json:"track"`
ExerciseSlug string `json:"exercise"`
ID string `json:"id"`
Team string `json:"team,omitempty"`
URL string `json:"url"`
Handle string `json:"handle"`
IsRequester bool `json:"is_requester"`
Expand Down Expand Up @@ -98,9 +97,6 @@ func (em *ExerciseMetadata) Exercise(workspace string) Exercise {

// root represents the root of the exercise.
func (em *ExerciseMetadata) root(workspace string) string {
if em.Team != "" {
return filepath.Join(workspace, "teams", em.Team)
}
if !em.IsRequester {
return filepath.Join(workspace, "users", em.Handle)
}
Expand Down
22 changes: 0 additions & 22 deletions workspace/workspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,28 +58,6 @@ func (ws Workspace) PotentialExercises() ([]Exercise, error) {
continue
}

if topInfo.Name() == "teams" {
subInfos, err := os.ReadDir(filepath.Join(ws.Dir, "teams"))
if err != nil {
return nil, err
}

for _, subInfo := range subInfos {
teamWs, err := New(filepath.Join(ws.Dir, "teams", subInfo.Name()))
if err != nil {
return nil, err
}

teamExercises, err := teamWs.PotentialExercises()
if err != nil {
return nil, err
}

exercises = append(exercises, teamExercises...)
}
continue
}

subInfos, err := os.ReadDir(filepath.Join(ws.Dir, topInfo.Name()))
if err != nil {
return nil, err
Expand Down
8 changes: 2 additions & 6 deletions workspace/workspace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,13 @@ func TestWorkspacePotentialExercises(t *testing.T) {
b1 := filepath.Join(tmpDir, "track-b", "exercise-one")
b2 := filepath.Join(tmpDir, "track-b", "exercise-two")

// It should find teams exercises
team := filepath.Join(tmpDir, "teams", "some-team", "track-c", "exercise-one")

// It should ignore other people's exercises.
alice := filepath.Join(tmpDir, "users", "alice", "track-a", "exercise-one")

// It should ignore nested dirs within exercises.
nested := filepath.Join(a1, "subdir", "deeper-dir", "another-deep-dir")

for _, path := range []string{a1, b1, b2, team, alice, nested} {
for _, path := range []string{a1, b1, b2, alice, nested} {
err := os.MkdirAll(path, os.FileMode(0755))
assert.NoError(t, err)
}
Expand All @@ -38,7 +35,7 @@ func TestWorkspacePotentialExercises(t *testing.T) {

exercises, err := ws.PotentialExercises()
assert.NoError(t, err)
if assert.Equal(t, 4, len(exercises)) {
if assert.Equal(t, 3, len(exercises)) {
paths := make([]string, len(exercises))
for i, e := range exercises {
paths[i] = e.Path()
Expand All @@ -48,7 +45,6 @@ func TestWorkspacePotentialExercises(t *testing.T) {
assert.Equal(t, paths[0], "track-a/exercise-one")
assert.Equal(t, paths[1], "track-b/exercise-one")
assert.Equal(t, paths[2], "track-b/exercise-two")
assert.Equal(t, paths[3], "track-c/exercise-one")
}
}

Expand Down
Loading