Add User view

[KevinKelly25]

I created this to show my work so far with the user view. With the exception of Lines 4 to 6, I tested all other statements, which was done in an independent database rather then ClassDB (so qualified object name, classdb. was not used and was added after testing).

I have never isMember before and while I did read how it was used in testHelpers.sql I am unsure If I used the proper syntax.

The push into dev will probably have to wait until other PRs are done. However, in the meantime, I would appreciate it if I can get some feedback.

[wildtayne]

This is a good start. I have a few comments:

• In the future, I think it would be better to not include commits from a separate branch that has not yet been merged into dev in a PR. Since all the changes @smurthys has made in add-RoleBase-mgmt are not in dev yet, they all show up when looking at the diffs in the files tab, which makes it hard to tell what changes are unique to this PR. In this situation, I think it would be better to keep your new changes in one branch, and create a integration test branch locally where you merge add-RoleBase-mgmt and your changes. This will allow a PR with only your changes, while still allowing you to test integration with the new code.
• The current situation won't cause any issues when merging the PR, it just makes its contents less clear.
• I'm not sure if we have made an official decision on it, but I think classdb.User should have ClassDB in title case. It seems like most of our new code is using this convention.
• I might be missing something, but ClassDB.User selects from ClassDB.User_t, which doesn't seem to exist. Based on its functionality, perhaps this should be ClassDB.RoleBase?
• ClassDB.IsMember() seems to be used correctly
• The CASE statements could maybe be more readable if they were split among multiple lines. I would be interested in hearing other's thoughts on how to format this view code in general, since I find large views like this pretty hard to format well.

[KevinKelly25]

@srrollo Thank you for the comment.

I know what you mean about the extra commits but I am not completely sure how to stop a pull request from including all the extra info. I will ask in the meeting today.

As for the use of User_t instead of User; I am using User_t because the convention in CS205. This convention is used when you have derived attributes in a table you create a view with the table name and add _t to the table name. This way when User is called it is referring to the view instead of the table which is normally what would be called since it has all the added derived attributes.

For the format concerning CASE I did try to make them look neater but most of the time they ended up getting more messy and harder to read. I would like to know as well if there is a better convention for the format that someone knows.

[smurthys]

It is OK to locally merge another ongoing branch to your own. In fact, It is often required to do this if the functionality we want to provide needs stuff in another branch. However, we should avoid pushing changes from our branch while the other branch is still outstanding.

(git compares files in your branch plus the changes from the other branch with the dev branch. Thus the many files and comments, etc. showing up in this PR.)

That said, just make sure you don't make any changes locally to files from the other branch.

On the changes themselves (echoing some of @srrollo's observations):

• The file addRoleBaseMgmtProxy.sql should be called addUserMgmtProxy.sql because its content should go into addUserMgmt.sql
• The base table should be ClassDB.RoleBase
• Most (all?) of the scalar-value sub-queries are unnecessary because they end up returning the same value the functions they internally call. See example below:

(SELECT CASE WHEN ClassDB.IsMember(RoleName , 'classdb_instructor') THEN TRUE ELSE FALSE END)

Is the same as just calling ClassDB.IsMember(RoleName , 'classdb_instructor') because the CASE statement returns TRUE if TRUE and FALSE if FALSE.

I recall you started a gist for this purpose. I recommend you use that and get feedback over there. Or, you could use the Winter2018 repo to cause notifications and get faster responses.

[smurthys]

I propose closing this PR and deleting the branch because the enclosed commits have been invalidated by other PRs since, e.g., #154

[KevinKelly25]

Closing PR, as discussed in meeting, because enclosed commits have been invalidated by other PRs since creation