DASSL / ClassDB

An open-source system to let students experiment with relational data
https://dassl.github.io/ClassDB/
Other
7 stars 2 forks source link

No functionality available to create a team (N) #218

Closed afig closed 6 years ago

afig commented 6 years ago

From an earlier version of M3's Wiki page:

afig commented 6 years ago

Functionality has been implemented along with basic tests in branch add-teams-functionality, however a change to createRole() may be needed (see ~Issue #223~ comments below)

afig commented 6 years ago

Having the fullName parameter for the future createTeam() be optional results in a conflict with the current implementation of createRole(). This is because createRole() raises an EXCEPTION if fullName is NULL or empty.

A few possible solutions are:

  1. Have createRole() not enforce a non-empty fullName if isTeam is true. This will require it to be ensured that allowing a NULL or empty name does not lead to other issues. The ClassDB Role-Creation Process Diagram will also have to be updated.

  2. Make fullName for teams required. Note that it may be slightly confusing for users to be required to enter two names for a team.

  3. Leave fullName "optional", but coalesce with teamName before calling createRole().

The first solution is ideal, and is likely not be a huge effort to implement (there would not be any API changes since the behavior only impacts teams, which were not officially supported in 2.0). Solutions 2 and 3 are left as "backups" in the event of time constraints or issues with the first solution. Current development of teams is using solution 3 as a temporary measure so as to not slow down development.

smurthys commented 6 years ago

Option 3 in @afig's list works, but should probably be implemented within function createRole so that function can be evolved later.

To be specific, createRole can use a team's role name as full name if the full name supplied is empty (after trimming) or NULL.

afig commented 6 years ago

I like @smurthys's suggestion about implementing option 3, but in createRole(). This opens up a couple of questions, however:

  1. In the event that createRole() does use a role name due to a NULL or empty full name, should a message be raised? What severity (NOTICE or WARNING) should be used? I would side with issuing a NOTICE, but would appreciate other perspectives on why or why not.

  2. Should this behavior only apply to teams? I would say yes to maintain backwards compatibility and avoid interface changes.

smurthys commented 6 years ago

Now that @afig mentions the potential of raising a notice, it might be worth going all in:

I believe these changes should work.

afig commented 6 years ago

Sounds good, thanks for investigating @smurthys. I will make these changes and verify that they are the only ones necessary.