-
Notifications
You must be signed in to change notification settings - Fork 69
Submissions rendering
Initially, c0d3-cli collects results of git diff master
from the end user and
send them to our server. User submissions are stored as strings in submissions
table.
See DiffView component. It gets diff
prop which is submission string from our
server. We're using gitDiffParser
library to parse a single string into
multiple files (which in turn are split into multiple
hunks).
Actual rendering is done by another library - react-diff-viewer. It gets a string and produces a jsx element (we're using a fork, for more details, see "Comments rendering" section below). It also has optional renderContent prop which is used for adding line comments. If you are considering switching to another diff rendering library keep in mind that without similar (string ⇾ jsx) function you will need to re implement comments from scratch.
DiffView keeps commentsState which tracks every line comment. It's an object
where every field is submissionId:fileName
, so every file could be uniquely
identified. For example:
{
"3:foobar.js":{lines:[],comments:[]},
"3:bazfoo.js":{lines:[], comments:[]}
}
Is a state of submission with id 3
, two files foobar.js
, bazfoo.js
and no
comments. When react-diff-viewer renders submissions it applies renderContent
(which was initially designed for code highlighting) function to every line.
We're checking if line number is present in the state and render CommentBox
under this line. By default renderContent only gets a line string as argument
that's why we forked it and added line number to arguments.
Another example. Initial state is:
{
"3:foobar.js":{lines:[3],comments:[{line:3, content:"Test comment"}]},
"3:bazfoo.js":{lines:[], comments:[]}
}
When react-diff-viewer reaches line 3
for foobar.js
it adds CommentBox with
a single comment saying Test comment
.
Unsurprisingly, adding a new comment means pushing its line number to the state.
Potential improvements:
See CommentBox component. It gets comments array and a bunch of props which are
used to uniquely identify every comment. There is local state hidden
which
hides conversations on click. It doesn't persist on rerender, and if you refresh
the page, every conversation is open again.
When you click on 'Add Comment' you trigger addComment mutation. For those unfamiliar with Apollo, mutations only modify server state, and you need to update client's cache manually. To do this, we're using updateCache helper. It reads the whole GET_PREVIOUS_SUBMISSIONS query from cache, finds submission with the given id, adds a new comment to it and writes it back to cache. When cache is updated, it triggers rerender, and a new comment appears on your screen.
Potential improvements:
-
Use optimistic response for adding a comment. This way you don't need to wait for round trip to our server and back.
-
Make conversation state persistent. Issue.
-
Modify only one submission instead of rewriting the entire query.
See ChallengeMaterial and ReviewCard components (if you're reading this for the first time, it's easier to start with ReviewCard).
Initially, user submissions are part of GET_APP query for student and GET_SUBMISSIONS query for reviewer. GET_APP is pretty much always in cache while the review page is hidden behind the loading spinner.
Querying for previous submissions is non-blocking, we're using a separate GET_PREVIOUS_SUBMISSIONS query and new SelectIteration component which has three states (loading, error and complete).
Reviewer and ChallengeMaterial components keep local submissions and index
states. Initially index is -1
(we don't know how many submissions the user
has). ReviewerCard has one useEffect to update its state when
GET_PREVIOUS_SUBMISSIONS query comes back (or if local cache was rewritten by
adding new comment). ChallengeMaterial needs another effect in case of user
selects a different challenge (curriculum page renders only one submission at a
time).
Potential improvements:
- Refactor ChallengeMaterial. Split into smaller components and maybe use context to avoid unnecessary prop drilling.
- Add color highlighting to previous iterations. Issue.