Topics - List, add, edit, delete, sort; Go test case updates; Common function to redirect to home #4
Conversation
ankurcdoshi
commented
Dec 10, 2024
ankurcdoshi
commented
- Topics - List, add, edit, delete, sort
- Redirection to home using common function
- Chapter service removed. Its function is now generalized in common Service.
- Update success message template made generic to handle both chapter & topic update
- Added singleton HTMXMiddleware to fix failing go test cases
- Added new go test cases related to navigation handler registrations
…mon function 1. Chapter service removed. Its function is now generalized in Service. 2. Load, add, edit, delete topic 3. Reusable function to redirect to home page when direct loading of specific url is not possible 4. Update success message template made generic to handle both chapter & topic update
1. Added singleton HTMXMiddleware to fix failing go test cases 2. Added new go test cases related to navigation handler registrations
ankurcdoshi
requested review from
pritamps,
deepansh96,
suryabulusu and
Bahugunajii
Readded the tab ids. Although those were not used in actual code, still tests were using them.
| .sub-tab:focus { | ||
| @apply text-white bg-red-400 rounded; | ||
| } | ||
| .sub-tab:hover { | ||
| @apply text-white bg-red-400 rounded; | ||
| } |
There was a problem hiding this comment.
Looks like redundant styles. What's the purpose?
There was a problem hiding this comment.
These are used on single chapter screen having sub-tabs for topics, tests, resources & passages.
internal/handlers/chapter_handler.go
Outdated
| type TopicsData struct { | ||
| ChapterId string | ||
| TopicsSortState SortState | ||
| } |
There was a problem hiding this comment.
Let's maintain type definitions in a separate file
There was a problem hiding this comment.
Shifted to separate file in dto package
| @@ -151,16 +174,19 @@ func (h *ChaptersHandler) UpdateChapter(w http.ResponseWriter, r *http.Request) | |||
| chapterName := r.FormValue("name") | |||
| chapterCode := r.FormValue("code") | |||
|
|
|||
| _, err = h.chaptersService.UpdateChapter(chapterIdStr, chapterCode, chapterName, chaptersKey, | |||
| dummyChapterPtr := &models.Chapter{} | |||
There was a problem hiding this comment.
What is this dummyChapterPtr? Can't we create a chapterMap without this?
There was a problem hiding this comment.
We can create chapterMap without dummyChapterPtr, but then we cannot have multiple functions with same BuildMap() name - one in Chapter model and another in Topic model, instead we will have to name them something like BuildChapterMap(), BuildTopicMap().
internal/handlers/generic_handler.go
Outdated
| type HTMXMiddleware struct { | ||
| handler http.Handler | ||
| lock sync.RWMutex | ||
| } |
There was a problem hiding this comment.
Let's move type definitions in a separate file
There was a problem hiding this comment.
Shifted to separate file htmx_middleware in middleware package.
internal/handlers/generic_handler.go
Outdated
| func (m *HTMXMiddleware) ServeHTTP(w http.ResponseWriter, r *http.Request) { | ||
| m.lock.RLock() | ||
| defer m.lock.RUnlock() | ||
|
|
||
| // Check if the request is from HTMX | ||
| if r.Header.Get("HX-Request") == "" { | ||
| // If the request is NOT from HTMX, redirect to the home page | ||
| http.Redirect(w, r, "/", http.StatusSeeOther) | ||
| return | ||
| } | ||
|
|
||
| // Call the next handler if set | ||
| if m.handler != nil { | ||
| m.handler.ServeHTTP(w, r) | ||
| } | ||
| } |
There was a problem hiding this comment.
The code is difficult to understand. Could you rename variables to improve clarity, such as:
w → responseWriter
r → request
m → middleware
This would make the code more readable.
| if (!selectedValue) { | ||
| // if dropdown selection is not known from url then get it from sessionStorage | ||
| selectedValue = sessionStorage.getItem(sessionStorageKey); | ||
| } |
There was a problem hiding this comment.
What if sessionStorageKey is also not there
There was a problem hiding this comment.
In that case getItem() will return null and hence next condition won't pass resulting in no change in dropdown selection.
| <td></td> | ||
| <td></td> | ||
| <td></td> | ||
| <td></td> | ||
| <td></td> | ||
| <td></td> |
There was a problem hiding this comment.
Not sure yet what to put over there. These columns are there on old CMS, and copied as it is with just column names but without data as of now.
web/html/topic_row.html
Outdated
| <td></td> | ||
| <td></td> | ||
| <td class="space-x-4"> | ||
| <button class="text-blue-500 hover:text-red-500 hover:scale-110" hx-get="/edit-topic?id={{.ID}}" |
There was a problem hiding this comment.
This class is used in multiple places. Let's create a common class and reuse it everywhere for consistency.
There was a problem hiding this comment.
Added common css class action-button and used it at all 5 places.
web/static/css/output.css
Outdated
There was a problem hiding this comment.
What's the reason for using both Tailwind and CSS? Can't we just use Tailwind alone?
There was a problem hiding this comment.
input.css is added for custom styles which generates single file output.css having both tailwind styles as well as our custom styles. So our code refers to only single css file - output.css
1. HTMXMiddleware shifted to middleware package 2. Added css class action-button to reuse it at multiple places 3. dto package added to separate out dto type definitions 4. References w, r and m renamed to responseWriter, request and middleware, respectively
Singleton middleware replaced by one object per request
Check if go server is running, retry 5 times if not running with 2 seconds delay before each retry. If still doesn't run then exit workflow.
1. netstat -tulnp → Lists active ports to confirm if :8080 is open. 2. ps aux | grep main → Checks if the Go server process is actually running. 3. cat server.log → Prints Go server logs if it fails to start.
Added temporary .env file creation from github secrets just for playwright tests
1. Stop go server on github after tests are completed 2. Remove code used for - debugging and retry connecting
