Merge branch 'master' of https://github.com/nus-cs3281/2024

nus-cs3281 · Apr 26, 2024 · 2ab1675 · 2ab1675
2 parents d6c9f7b + e64682a
commit 2ab1675
Show file tree

Hide file tree

Showing 77 changed files with 2,510 additions and 211 deletions.
diff --git a/cs3282-index.md b/cs3282-index.md
@@ -9,29 +9,29 @@ pageNav: 2
     {name: 'CATcher', students: [
         ['GOH YEE CHONG, GABRIEL', 'gycgabriel', 'A1', 'B1', 'C1'],
         ['LEE XIONG JIE, ISAAC', 'luminousleek', 'A1', 'B1', 'C1'],
-        ['VIGNESH SANKAR IYER', 'vigneshsankariyer1234567890', 'A3', 'B3', 'C3'],
+        ['VIGNESH SANKAR IYER', 'vigneshsankariyer1234567890', 'A3', 'B3', 'C2'],
         ['WONG CHEE HONG', 'cheehongw', 'A2', 'B2', 'C2']
     ]},
     {name: 'MarkBind', students: [
         ['CHAN YU CHENG', 'yucheng11122017', 'A1', 'B1', 'C1'],
         ['ELTON GOH JUN HAO', 'EltonGohJH', 'A1', 'B1', 'C1'],
-        ['HANNAH CHIA KAI XIN', 'kaixin-hc', 'A3', 'B3', 'C3'],
-        ['LEE WEI, DAVID', 'itsyme', 'A3', 'B3', 'C3']
+        ['HANNAH CHIA KAI XIN', 'kaixin-hc', 'A3', 'B3', 'C2'],
+        ['LEE WEI, DAVID', 'itsyme', 'A3', 'B3', 'C2']
     ]},
     {name: 'RepoSense', students: [
-        ['CHARISMA KAUSAR', 'ckcherry23', 'A3', 'B3', 'C3'],
-        ['DAVID GARETH ONG', 'vvidday', 'A3', 'B3', 'C3'],
-        ['GOKUL RAJIV', 'gok99', 'A2', 'B2', 'C2'],
-        ['MARCUS TANG XIN KYE', 'MarcusTXK', 'A2', 'B1', 'C2']
+        ['CHARISMA KAUSAR', 'ckcherry23', 'A3', 'B3', 'C2'],
+        ['DAVID GARETH ONG', 'vvidday', 'A3', 'B3', 'C2'],
+        ['GOKUL RAJIV', 'gok99', 'A2', 'B2', 'C1'],
+        ['MARCUS TANG XIN KYE', 'MarcusTXK', 'A2', 'B1', 'C1']
     ]},
     {name: 'TEAMMATES', students: [
         ['CHANG WENG YEW, NICOLAS', 'Nicolascwy', 'A2', 'B2', 'C2'],
-        ['DOMINIC LIM KAI JUN', 'domlimm', 'A2', 'B2', 'C2'],
+        ['DOMINIC LIM KAI JUN', 'domlimm', 'A2', 'B2', 'C1'],
         ['JAY ALJELO SAEZ TING', 'jayasting98', 'A1', 'B1', 'C1'],
-        ['KEVIN FOONG WEI TONG', 'kevin9foong', 'A3', 'B3', 'C3'],
+        ['KEVIN FOONG WEI TONG', 'kevin9foong', 'A3', 'B3', 'C2'],
         ['MOK KHENG SHENG FERGUS', 'FergusMok', 'A1', 'B1', 'C1'],
-        ['NEO WEI QING', 'weiquu', 'A2', 'B2', 'C2'],
-        ['ONG JUN HENG, CEDRIC', 'cedricongjh', 'A3', 'B3', 'C3'],
+        ['NEO WEI QING', 'weiquu', 'A2', 'B2', 'C1'],
+        ['ONG JUN HENG, CEDRIC', 'cedricongjh', 'A3', 'B3', 'C2'],
         ['SIM SING YEE, EUNICE', 'EuniceSim142', 'A1', 'B2', 'C1'],
         ['ZHANG ZIQING', 'ziqing26', 'A2', 'B2', 'C2']
     ]}

diff --git a/students/Arif-Khalid/knowledge.md b/students/Arif-Khalid/knowledge.md
@@ -91,3 +91,41 @@ Code quality is always important but is especially so when there are so many peo
 I learned about code quality through analysing the responses of seniors to my own pull requests as well as other's pull requests, supplementing my knowledge by reading articles on code quality both generally and specific to web development
 * Inspection of pull requests gave me understanding of what is good quality code and what is considered bad along with the reasoning behind those decisions
 * Articles online provided me with more general guidelines pertaining to code quality in large projects, helping fill in the gaps that I didnt encounter in PR reviews
+
+### Testing
+Testing is another important part of any project as it reduces the occurrence of major errors and bugs throughout development. With little prior experience in testing, I sought to learn more about it and apply it in WATcher.
+* Jasmine
+  * A testing framework for javascript
+  * Clean and intuitive syntax
+  * Suite of functionality developed over many years
+  * I learned Jasmine through looking through test cases in CATcher and WATcher, along with reading its official documentation
+    * ``describe(string, function)`` houses related specs labeled by string and defined as function
+    * ``it(string, function)`` defines a spec labeled by string and defined as function
+    * ``expect(any).toBeFalse`` defines an expectation of ``any``. There are a large number of matchers for any possible comparison
+    * ``beforeEach(function)`` defines a function to be called before each of the specs in this describe block
+    * ``createSpyObj(string, string[])`` creates a spy object that acts as a stub for classes that are depended on by what is being tested. Spies can track calls to it and all arguments
+* Test case design
+  * Boundary Value Analysis and equivalence partitioning
+    * Boundary value analysis is a technique where tests are designed to include representatives of boundary values in a range
+    * Equivalence partitioning is a technique where input data is partitioned into units of equivalent data for which tests can be written
+    * These techniques allow for a smaller number of tests to be written, for essentially the same amount of coverage
+      * This is because inputs which would fail/pass for the same reason, such as being an input of an invalid type, are grouped as a single or only a few test cases.
+      * The alternative would be to create tests for each input type in this example, straining developer resources for not much benefit
+  * Testing for behaviour
+    * A common mistake is to test for implementation rather than behaviour
+    * This would result in failed test cases when implementation changes even though the resulting behaviour, what the user would experience, remains the same
+    * Test cases should test for what the result is versus what the implementation is
+    * An example would be testing whether a variable changes in component A correctly vs testing what other components receive from component A after the change
+    * A developer might edit the implementation of component A so the variable no longer changes, however the accurate behaviour of emission to other components remains the same and the test cases should not fail
+  * Testing coverage
+    * Test coverage is how much of the code has actually been ran through during testing
+      * Function/method coverage : based on functions executed e.g., testing executed 90 out of 100 functions
+      * Statement coverage : based on the number of lines of code executed e.g., testing executed 23k out of 25k LOC
+      * Decision/branch coverage : based on the decision points exercised e.g., an if statement evaluated to both true and false with separate test cases during testing is considered 'covered'
+      * Condition coverage : based on the boolean sub-expressions, each evaluated to both true and false with different test cases
+    * A good future implementation would be to implement code coverage as a github action report when making pull requests to main
+    * At the very least, all public functions of a class should be uniquely tested in order to verify behaviour seen by other components
+I learned about testing web applications through Nereus, reading Jasmine documentation, articles and youtube videos about testing and the [CS2113 website](https://nus-cs2113-ay2324s2.github.io/website/index.html)
+* Nereus imparted knowledge of testing which helped me understand the core fundamentals, allowing me to more quickly pick up the technique as I learnt, especially the test case implementation
+* The Jasmine documentation gave me confidence in creating my own test cases for unique behaviour such as changing routes in testing
+* Youtube videos, articles and the CS2113 website helped me to understand and implement test case design techniques to create comprehensive and well designed test cases
diff --git a/students/Arif-Khalid/progress.md b/students/Arif-Khalid/progress.md
@@ -1,3 +1,17 @@
+# Key Contributions
+* Actively participated in submitting and resolving issues found, implementing features and team discussions
+* Focus on bug fixes that hurt the usability of CATcher and WATcher
+* Reviewed PRs constructively with focus on scalability and code style of the existing code base
+* Update CI/CD workflow, specifically improving automation by using an on push deployment and a release drafter template and bot which formatted and displayed release changes  
+  [Automatic deployment #272](https://github.com/CATcher-org/WATcher/pull/272), [Release changelog automation #285](https://github.com/CATcher-org/WATcher/pull/285)
+* Revamp and updates to filter system of WATcher
+  * Adopted a centralised service and observer pattern behaviour  
+    [Refactor certain filters into its own service #259](https://github.com/CATcher-org/WATcher/pull/259), [Refactor sorting #261](https://github.com/CATcher-org/WATcher/pull/261), [Refactor milestone filters #264](https://github.com/CATcher-org/WATcher/pull/264)
+  * Update filters to be stored into and restored from url  
+    [Add filters to url #314](https://github.com/CATcher-org/WATcher/pull/314)
+  * Allow filters to be kept across repo changes  
+    [Keep filters when switching repo #281](https://github.com/CATcher-org/WATcher/pull/281)
+
 | Week | Achievements |
 | ---- | ------------ |
 | 2 | Merged PR: [Hide 0 issue columns #223](https://github.com/CATcher-org/WATcher/pull/223) |
@@ -6,17 +20,48 @@
 | 5 | Merged PR: [Remove unused services #238](https://github.com/CATcher-org/WATcher/pull/238), Merged PR: [Remove unused models #253](https://github.com/CATcher-org/WATcher/pull/253), Merged PR: [Remove unused session-fix-confirmation component #250](https://github.com/CATcher-org/WATcher/pull/250) |
 | 5 | Reviewed PR: [Upgrade to angular 11 #252](https://github.com/CATcher-org/WATcher/pull/252) |
 | 5 | Reviewed PR: [Refactor Label model #254](https://github.com/CATcher-org/WATcher/pull/254) |
+| 5 | Merged PR: [Faulty list view when back navigating #1243](https://github.com/CATcher-org/CATcher/pull/1243) |
 | 6 | Reviewed PR: [Add shareable repo-specific URL #255](https://github.com/CATcher-org/WATcher/pull/255) |
 | 6 | Merged PR: [Refactor certain filters into its own service #259](https://github.com/CATcher-org/WATcher/pull/259) |
 | 6 | Merged PR: [Refactor sorting #261](https://github.com/CATcher-org/WATcher/pull/261) |
 | 6 | Merged PR: [Refactor milestone filters #264](https://github.com/CATcher-org/WATcher/pull/264) |
-| 7 | Merged PR: [Refactor title filter #265](https://github.com/CATcher-org/WATcher/pull/265) |
 | 7 | Reviewed PR: [Upgrade to angular 12 #267](https://github.com/CATcher-org/WATcher/pull/267) |
 | 7 | Reviewed PR: [Fix zone testing import error #269](https://github.com/CATcher-org/WATcher/pull/269) |
-| 7 | Merged PR: [Automatic deployment #272](https://github.com/CATcher-org/WATcher/pull/272) |
 | 7 | Reviewed PR: [Update test cases for phase service #275](https://github.com/CATcher-org/WATcher/pull/275) |
 | 7 | Reviewed PR: [Three-state labels #282](https://github.com/CATcher-org/WATcher/pull/282) |
+| 7 | Merged PR: [Refactor title filter #265](https://github.com/CATcher-org/WATcher/pull/265) |
+| 7 | Merged PR: [Automatic deployment #272](https://github.com/CATcher-org/WATcher/pull/272) |
 | 7 | Submitted Issue: [Release changelog automation #273](https://github.com/CATcher-org/WATcher/issues/273) |
 | 7 | Submitted Issue: [Save milestones by name #283](https://github.com/CATcher-org/WATcher/issues/283) |
-| 7 | Pending PR: [Keep filters when switching repo #281](https://github.com/CATcher-org/WATcher/pull/281) |
-| 7 | Pending PR: [Release changelog automation #285](https://github.com/CATcher-org/WATcher/pull/285) |
+| 8 | Submitted Issue: [Incorrect numbering in user-workflow #35](https://github.com/CATcher-org/catcher-org.github.io/issues/35) |
+| 8 | Submitted Issue: [Add tests for filters service #295](https://github.com/CATcher-org/WATcher/issues/295) |
+| 8 | Reviewed PR: [Update Design page #14](https://github.com/CATcher-org/WATcher-docs/pull/14) |
+| 8 | Reviewed PR: [Enable pre push hook for npm run test #288](https://github.com/CATcher-org/WATcher/pull/288) |
+| 8 | Reviewed PR: [Remove sorting by assignees in issue sorter #286](https://github.com/CATcher-org/WATcher/pull/286) |
+| 8 | Merged PR: [Refactor milestones to save by name #289](https://github.com/CATcher-org/WATcher/pull/289) |
+| 9 | Submitted Issue: [Label filter bar test case error #305](https://github.com/CATcher-org/WATcher/issues/305) |
+| 9 | Reviewed PR: [Hide redundant column pagination #309](https://github.com/CATcher-org/WATcher/pull/309) |
+| 9 | Reviewed PR: [Status filter checkboxes #310](https://github.com/CATcher-org/WATcher/pull/310) |
+| 9 | Merged PR: [Keep filters when switching repo #281](https://github.com/CATcher-org/WATcher/pull/281) |
+| 9 | Merged PR: [Release changelog automation #285](https://github.com/CATcher-org/WATcher/pull/285) |
+| 10 | Reviewed PR: [Integrate Grouping Service #313](https://github.com/CATcher-org/WATcher/pull/313) |
+| 10 | Merged PR: [Keep milestones when switching repo #311](https://github.com/CATcher-org/WATcher/pull/311) |
+| 10 | Merged PR: [Add filters to url #314](https://github.com/CATcher-org/WATcher/pull/314) |
+| 11 | Submitted Issue: [Default preset view is custom when it should be currently active #333](https://github.com/CATcher-org/WATcher/issues/333) |
+| 11 | Submitted Issue: [Remove quotations from filters in url #340](https://github.com/CATcher-org/WATcher/issues/340) |
+| 11 | Submitted Issue: [Showing of preset views before selecting a repo](https://github.com/CATcher-org/WATcher/issues/352) |
+| 11 | Merged PR: [Fix default preset view #334](https://github.com/CATcher-org/WATcher/pull/334) |
+| 11 | Merged PR: [Remove quotation marks from url #345](https://github.com/CATcher-org/WATcher/pull/345) |
+| 11 | Reviewed PR: [Include groupby params in url #319](https://github.com/CATcher-org/WATcher/pull/319) |
+| 11 | Reviewed PR: [Add preset views #320](https://github.com/CATcher-org/WATcher/pull/320) |
+| 11 | Reviewed PR: [Update repo on back and forward navigation #322](https://github.com/CATcher-org/WATcher/pull/322) |
+| 11 | Reviewed PR: [Reset GroupingContextService only if "keep filter" is selected #324](https://github.com/CATcher-org/WATcher/pull/324) |
+| 11 | Reviewed PR: [Create release 1.2.0 #327](https://github.com/CATcher-org/WATcher/pull/327), Reviewed PR: [Resolve conflicts for 1.2.0 #329](https://github.com/CATcher-org/WATcher/pull/329), Reviewed PR: [Deploy V1.2.0 #331](https://github.com/CATcher-org/WATcher/pull/331) |
+| 11 | Reviewed PR: [Create release V1.2.1 #349](https://github.com/CATcher-org/WATcher/pull/349), Reviewed PR: [Deploy V1.2.1 #350](https://github.com/CATcher-org/WATcher/pull/350) |
+| 11 | Reviewed PR: [Optimise Github API calls #360](https://github.com/CATcher-org/WATcher/pull/360) |
+| 12 | Submitted Issue: [Reset of sort filter after some time #372](https://github.com/CATcher-org/WATcher/issues/372) |
+| 12 | Merged PR: [Show preset view only when repo is set #355](https://github.com/CATcher-org/WATcher/pull/355) |
+| 12 | Merged PR: [Fix top and bottom shadow of columns #357](https://github.com/CATcher-org/WATcher/pull/357) |
+| 12 | Merged PR: [Create release V1.2.2 #364](https://github.com/CATcher-org/WATcher/pull/364), Merged PR: [Deploy V1.2.2 #365](https://github.com/CATcher-org/WATcher/pull/365) |
+| 12 | Merged PR: [Fix reset of filters on label fetch #374](https://github.com/CATcher-org/WATcher/pull/374) |
+| 13 | Pending PR: [Add test cases for filters service #312](https://github.com/CATcher-org/WATcher/pull/312) |
diff --git a/students/EltonGohJH/observations.md b/students/EltonGohJH/observations.md
@@ -6,10 +6,8 @@ Mattermost is an open-source collaboration platform designed for secure communic
 ### Twenty
 Twenty CRM is a modern, open-source Customer Relationship Management (CRM) platform. It serves as an self-hostable alternative to Salesforce.
 
-
 ### My Contributions
-
-In the [mattermost PR (approved not merged yet)](https://github.com/mattermost/mattermost/pull/26278). I addressed this [issue](https://github.com/mattermost/mattermost/issues/25991) where the CLI command to list the teams uses a magic number of 9999. Utilizing such large magic numbers presents two problems: it restricts the ability to list more than 9999 teams and could result in a request that is too large. To solve this, I implemented pagination for the request, with each page containing 200 teams. Subsequently, I updated the test cases to reflect the new expected behavior.
+In the [mattermost PR (merged)](https://github.com/mattermost/mattermost/pull/26278). I addressed this [issue](https://github.com/mattermost/mattermost/issues/25991) where the CLI command to list the teams uses a magic number of 9999. Utilizing such large magic numbers presents two problems: it restricts the ability to list more than 9999 teams and could result in a request that is too large. To solve this, I implemented pagination for the request, with each page containing 200 teams. Subsequently, I updated the test cases to reflect the new expected behavior.
 
 In the [Twenty PR (merged)](https://github.com/twentyhq/twenty/pull/4198). I addressed an [issue](https://github.com/twentyhq/twenty/issues/4181) reported by a user concerning LinkedIn school URLs not parsing correctly. Upon investigating the issue on the frontend, I discovered that the existing regex was only configured to support company URLs. To resolve this, I updated the regex to also accommodate school URLs and conducted tests to ensure the fix was effective.
 
@@ -29,3 +27,9 @@ Overall, it is a good experience as I learnt more alternatives to Lerna and NPM
 
 #### Resource used:
 - [Yarn workspaces docs](https://yarnpkg.com/features/workspaces)
+
+### Practices/tools from Mattermost that could be adopted by Markbind
+
+I was particularly impressed with the [Twenty's onboarding guide](https://docs.twenty.com/start/local-setup/) because it includes multi-OS setup guides and instructions on setting up through Docker containers. Furthermore, it provides an IDE setup guide, and its repository contains a `.vscode/extensions.json` file that assists users in configuring VS Code. For Markbind, while the Docker container setup may not be necessary, adopting a multi-OS guide could be beneficial. It could promote useful tools like `nvm` for testing across multiple Node.js versions, and a VS Code extensions list could help new developers adhere to our coding practices.
+
+I was really impressed with the PR review workflow at Mattermost. It's incredibly systematic, featuring stages such as UI review, Dev review, and QA review, which make the process feel seamless. Additionally, they utilize bots to remind reviewers to complete their reviews. While Markbind is smaller and might not require such an elaborate setup, investigating the potential of GitHub PR bots could be beneficial. These tools could streamline our review process and ensure that contributions are efficiently and effectively vetted.