inspectorg4dget / anonymousfeedback Goto Github PK

Sub of #11

Pull feedback for course

This is a feature mentioned by @NuclearBanane today, with @inspectorG4dget aware of it.

Support should be added for sections with multiple TAs; this is because a section may have a DGD and a lab taught by two TAs.

The view feedback feature is broken with the new schema changes. please fix

There's currently some confusion about whether an interface refers to a course section or a DGD section. A glaring test case for this can be reproduced with the following steps:

• As a student, select a course.

• You will be given a radio button group with DGD group identifiers (normally T1, T2, etc. using the test data).

• After selecting a group, submit some feedback.

Then:

• As a professor, select "View feedback."

• Select a course, and then a section (which is denoted using T1, T2, etc.).

• The database query returns feedback results for TAs in all available sections.

I'm not sure whether this is the result of an erroneous database query, or a schema which fails to disambiguate between class and DGD sections -- but I'd like some guidance on this to determine whether a fix is necessary (or if this is desired behaviour), and what kind of fix is needed, if any.

Currently, the /submitFeedback endpoint raises a 500 error on POST because of a type conflict on the pg8000 layer. This restricts testing related to #29 and should be resolved immediately as it impairs the progress of #29.

Within the professor view, create section feature

sub of #11

There is a request for a minor fix by @inspectorG4dget to order the results of /getCourses endpoint queries alphabetically.

This can be implemented as either an ORDER BY specifier in dbhandler.py or as a sort call in either dbhandler.py or in backend.py.

To preserve the role of backend.py as a data presentation and packaging layer (rather than a data retrieval layer), this functionality will be offloaded there.

Create an Endpoint on 'r/getSectionTA' which receives

{
    course :"ITI1120"
    section:"B1"
}

and returns

{
tas:[
    {
        name : "Ashwin Pancakes",
        taID : "1"
    },
    {
        name : "Paulina Meh",
        taID : "2"
    }
]
}

The @asynchronous decorator is meant for handler methods which should, e.g., perform actions while a database query is carried out.

At a glance, no methods in backend.py actually benefit from the use of @aynchronous, and block pending database output (which is generally returned fast enough that it makes very little difference whether or not @asynchronous is used).

This isn't a high-priority issue, but should be considered as time is made available for code cleanup and reorganization.

Using the current master:HEAD (5e692f2) and the included bootstrapping file (db_setup.sql) and testing data (db_test_data.sql), the application fails to load time and section information for classes ITI1120 and CEG2136. This results in no feedback form being displayed for either of these courses.

The cause of this issue requires diagnosis.

This problem does not affect ITI1121 from the testing data.

There's currently very little input validation being done server-side. For example, it's possible to submit an empty string when adding a course (as a professor); this will lead to an empty string being appended to the dropdown when attempting to fetch feedback (in addition to propagating to the student view). This should at the very least fail silently by being discarded by the server before a database hit.

Not immediately important or particularly high-priority, but it could add something nice to show off at the demo if there's time to implement before then.

The testing data should reflect an edge case mentioned by @verandaguy in #34 to test the code merged into master in #40. This means testing against cases where:

• There exist two DGD sections of the same class, occurring at the same time, with different TAs.

This models cases where two different DGD sections are held at the same time, but in different rooms.

While very unlikely to cause issues, it's best to integrate this test for thoroughness.

Note: this appears unrelated to #36.

If a course code is entered under Add course and Add section is selected from the dropdown, a pg8000.ProgrammingError is raised. The source of this issue is dbhandler.py:40, inside getCourses():

t.execute("""SELECT * FROM course""")

There's currently no server-side logging besides Tornado's own, which could stand to be more useful.

Server-side logging should be integrated using some widely-available logging module. logging itself may be overkill, but slog is always an option.

There's an immediately relevant issue in dbhandler.getCourseFeedbacks where a for loop attempts to unpack an 8-element list into 10 elements.

The problematic elements are those representing the start and end times of a section.

Upon inspection, these two problem values may not be consequential in the Professor view.

RFC: should these two values be preserved (which would require changes to the DB query in this function)?

sub of #11
Make feature for creating TA's within the Professor view

Related to #5
The section ID, such as ITI1120B01 where B01 is the section ID needs to be returned.

This means its needs to be added as a field in the DB.

This issue is suspected to be related to #33.

Using the current master:HEAD (5e692f2) as well as the provided database bootstrapping file and testing data, the application displays duplicates of tutorial sections when attempting to evaluate a TA for course ITI1121. Related screenshot attached.

The cause of this issue requires further investigation.

I hate UI. Someone make a good UI.

plz tks

How will we allow professors to add TAs, Sections and Course codes to the system?

This is a sub of this issue #5

Make the following functional improvements to the form:

• Make the form submit on the user clicking submit
• Ensure the user has filled out all fields expect comments fields
• Make form send course and section information (to remain restful)

This will give 2 options, Professor and Student.
This will be handled by / handler

This task is referenced in #2

This should be implemented either as a build hook (ahead-of-time; static assets live on the server as minified) or as a Tornado mixin (readily available; assets are minified and cached as such).

Noticeable performance gains can be had once this is implemented on JS and CSS files.

This isn't a necessary goal for the EECS demo, and will be sidelined while higher-priority issues are tackled; however, this is an important enhancement as this application nears a public rollout, and shouldn't be overlooked long-term.

Should we have a step by step process where the user sends an input, the server responds with a new list OR should we populate an HTML dropdown only?

In the original implementation, the TA assignment mechanism was incomplete client-side. This is to be implemented after major parts of #12 are complete.

The interface should consist of, at least:

• A dropdown for course selection;

• A dropdown for section selection; and

• A dropdown for TA selection.

Comments are welcome about additional features which may be useful.

Because of its limited scope and relatively simple API, writing unit tests for this application should be a high post-demo priority.

Ideally, these tests should be automated (if possible without expense or too much effort), or at least integrated into pre-commit hooks.

Currently, the UI does not update if a transaction fails or succeeds. This should be implemented as visual feedback is a major UI element in this application.

Make it so the form makes a submission to /submit which adds to the database

Related to #5

The name of the TA needs to be pulled and displayed

There are currently a few endpoints -- mostly PUT/POST -- which, while they invoke side-effects, don't finish and return a response to the client.

This makes it impossible to evaluate the success or failure of a query client-side and to update the view to reflect this. It can also result in long-running function invocations client-side before a timeout is detected.

These methods should include calls to self.finish().

The database schema enforces a first and last name when inserting new TAs. The new client-side only collects a first name, with no reliable way of extracting a first and last name separately.

The client should be updated to send the first and last names separately.

Currently, a student may only submit feedback for a DGD section once; subsequent attempts result in an error linked to a database constraint.

Expected behaviour is to have older feedback for a DGD section by the same student to be ovewritten with new feedback.

The Professor view allows you to:

• Add TA's
• Add Course's
• Add Sections
• View Feedback
• • Based on TA
• • Based on Section
• • Based on Course

Make the DBHandler use a config file, include an example config file, then make a commit adding it to the gitignore

The questions on feedback that need to be stored are the following.
The following are within a range of 1,5:

• How often was the TA prepared?
• How well did the TA communicate the Material?
• How relevant was the (lab/dgd) material to the class (Possible improvement on separating labs and dgds?)

The plan is to associate these into an array where all values range between 1 and 5.

The following are comment questions and require the student to write out an answer.:

• Any comments on the TA?
• What are your thoughts on this course?

Would be good to start figuring out how many range questions we'll have since ti does effect the schema.

Feel free to add more information below in comments.

Currently, the API does not conform to RESTful semantics. An example of this is the /getSections URI, which features a name which specifies a GET endpoint (in the name), but only supports POST methods.

This would be more appropriately be a GET endpoint at /sections.

There are multiple examples of this issue across backend.py.

Currently, there is no reliable way to authenticate student users of the application. Assuming uOttawa has a standard OAuth2.0 setup with their Google-provided email accounts, this could be a very reliable option for authentication.

Investigate whether this solution presents major issues and evaluate the amount of work required to implement this feature.

The current way of running this app involves simply launching it with Python. If the TTY running the python process sends SIGHUP, the application is interrupted.

Workarounds for this are currently:

• To launch the application in a multiplexer, like tmux or GNU screen, and then disconnect;
• To launch the application using nohup; this leaves the application in the foreground
• To launch the application using nohup and &, which puts the application largely in the background but makes it easy to "lose"

Integrating with an init system would involve creating a systemd script for this, which would take a few minutes of time.

How will we model the user interaction from / and inwards?

Within the professor view, allow the professor to create courses.

sub of #11

Currently, the app will allow feedback on sections without verifying if they have a TA

Would recommend doing #8 and #9 first since they will require revising the Schema

Currently, most of the code and API are undocumented. Critically, this includes very small comment coverage, and a lack of JSdoc and PyDoc coverage in the codebase.

The web API should likewise be documented.

As noted by @NuclearBanane in #36, some pg8000.ProgrammingErrors only appear if a previous cursor.execute() invocation resulted in an error.

These invocations should be structured in such a way that failing cursor.execute() invocations trigger a rollback.

sub of #11

Within the professor view, create feature to assign TA's to sections based on Course code and Section

The updated view now sends a form with data for multiple TAs if multiple are associated with a class.

Please describe the input JSON format you will be expecting.

This is a sub task of #5

Create a handler which parses through the form and enters it into the database. This requires :

• A handler on /submit
• Parsing al the ranges into an array
• Creating a DBHandler function for entering the parsed information into the database

Update feedback view to get TA's described in endpoint #23 and display an individual form for each.
Then it must asynchronously send the form feedback to the submitFeedback endpoint

As a professor, adding a section, a pg8000.ProgrammingError (normally caused by bad syntax) is raised when adding a course. The origin of this error is an invocation of cursor.execute() at dbhandler.py:89 inside createCourse():

t.execute("""INSERT INTO course(code) VALUES (%s)""", (courseCode,))

Possibly related to #29.

Using the Professor view, the application fails to display collected feedback, or acknowledge a lack thereof. This feature should maybe be implemented as an HTML table.

The root cause of this appears to be a bad unpacking of data fetched from the database by getCourseFeedbacks() in dbhandler.py.