FS-InfMath/Jobboerse
27
0
Fork 1
Code Issues 1 Pull requests Projects Releases 17 Wiki Activity

Redesign page + Webhook notifications #43

Merged
ldr merged 16 commits from facelift into main 2023-12-25 21:23:53 +01:00
Conversation 7 Commits 16 Files changed 42 +850 -299
ldr commented 2023-12-25 00:44:32 +01:00
Owner
Copy link
  • Redesign the website (see screenshots attached)
    • Change CSS
    • Update texts
      • Translate everything to German
      • Rephrase some texts
      • Fix typos and spelling mistakes
  • Add basic support for Discord webhooks
    • Add parameter webhook_url
    • If webhook_url is specified, simple messages including the title and company as well as a link back to the Jobbörse are sent whenever a job offer is made public (see screenshots attached)
- Redesign the website (see screenshots attached) - Change CSS - Update texts - Translate everything to German - Rephrase some texts - Fix typos and spelling mistakes - Add basic support for Discord webhooks - Add parameter `webhook_url` - If `webhook_url` is specified, simple messages including the title and company as well as a link back to the *Jobbörse* are sent whenever a job offer is made public (see screenshots attached)
Screenshot 2023-12-25 at 00-36-54 Joboffers.png
616 KiB
Screenshot 2023-12-25 at 00-36-28 Joboffers.png
723 KiB
Screenshot 2023-12-25 at 00-39-37 Successfully created Joboffer.png
346 KiB
Screenshot 2023-12-25 at 00-37-41 Create Joboffer.png
419 KiB
Screenshot 2023-12-25 at 00-40-53 Job Offer Confirmation.png
271 KiB
Screenshot 2023-12-25 at 00-36-54 Joboffers.png Screenshot 2023-12-25 at 00-36-28 Joboffers.png Screenshot 2023-12-25 at 00-39-37 Successfully created Joboffer.png Screenshot 2023-12-25 at 00-37-41 Create Joboffer.png Screenshot 2023-12-25 at 00-40-53 Job Offer Confirmation.png 
- fix spelling mistakes
- rephrase text
- translate all text to German
- update the FAQ
- Change font to sans-serif or FreeSans, if available
- Add clickable link to student association homepage
- Add hover effect to links in header
- Add theme colors from our logo
- Add tilted border to header
- Change students' association logo to the version with the university name
- translate the login screen to german
- fix a typo on the job offer creation screen
- update CSS
- Send messages when job offers are published
ldr self-assigned this 2023-12-25 00:58:28 +01:00
ldr requested review from ben 2023-12-25 00:58:33 +01:00
ben requested changes 2023-12-25 13:28:47 +01:00
ben left a comment
Copy link

Der Code sieht im allgemeinen gut aus, ich jab ein paar Anmerkungen hinterlassen.
Graphisch sieht es größtenteils auch gut aus,
ich finde den Header jedoch etwas überfüllt.

Der Code sieht im allgemeinen gut aus, ich jab ein paar Anmerkungen hinterlassen. Graphisch sieht es größtenteils auch gut aus, ich finde den Header jedoch etwas überfüllt.
Cargo.toml Outdated
@ -16,6 +16,7 @@ actix-files = "0.6.2"
actix-web = "4.3.0"
actix-session = { version = "0.7.2", features = ["cookie-session"] }
actix-multipart = "0.5.0"
awc ={ version = "3.2.0", features = ["openssl"] }
ben commented 2023-12-25 13:17:02 +01:00
Owner
Copy link

I try to use rustls for tls instead of openssl as it is annoying to get working on windows see the comment above the ldap3 dependency. Does awc support that as an alternative?
Also we now have two tls libraries in the dependencies which will bloat the binary and intermediate compile artefacts and we already had problems with the size when compiling on metabo as we only have ~2GB in /tmp.

I try to use rustls for tls instead of openssl as it is annoying to get working on windows see the comment above the ldap3 dependency. Does awc support that as an alternative? Also we now have two tls libraries in the dependencies which will bloat the binary and intermediate compile artefacts and we already had problems with the size when compiling on metabo as we only have ~2GB in /tmp.
ldr commented 2023-12-25 18:51:27 +01:00
Author
Owner
Copy link

Ok, I seem to have overlooked that comment. I have switched to rustls now.

Ok, I seem to have overlooked that comment. I have switched to rustls now.
ldr marked this conversation as resolved
packages/jobboerse/src/route/job_offer/review.rs
@ -44,0 +48,4 @@
company: entry.offering_party.to_owned(),
link: entry.highlight_link(&req)?,
};
crate::webhook::send_new_offer_message(&hb, webhook_url.to_owned(), &data)
ben commented 2023-12-25 13:21:19 +01:00
Owner
Copy link

Not aborting because the webhook failed is what I would expect, but it might be good to log the error instead of just ignoring it.

Not aborting because the webhook failed is what I would expect, but it might be good to log the error instead of just ignoring it.
ldr commented 2023-12-25 19:09:18 +01:00
Author
Owner
Copy link

This is now being logged directly inside the function.

This is now being logged directly inside the function.
ldr marked this conversation as resolved
packages/jobboerse/src/server_config.rs Outdated
@ -37,6 +37,7 @@ pub(crate) struct ProgramConfig {
pub(crate) login_provider: LoginProviderConfig,
#[serde(skip_serializing_if = "Option::is_none", default)]
pub(crate) email: Option<EmailConfig>,
pub(crate) webhook_url: Option<String>,
ben commented 2023-12-25 13:24:30 +01:00
Owner
Copy link

Add a #[serde(default)] attribute for the added field so that it will default to None if the field is absent, instead of failing to deserialize the config.

+    #[serde(default)]
     pub(crate) webhook_url: Option<String>,

Also if the webhook_url need to be a valid url url::Url might be better than String so that the validity is checked at deserialization time.

Add a `#[serde(default)]` attribute for the added field so that it will default to `None` if the field is absent, instead of failing to deserialize the config. ```diff + #[serde(default)] pub(crate) webhook_url: Option<String>, ``` Also if the `webhook_url` need to be a valid url `url::Url` might be better than `String` so that the validity is checked at deserialization time.
ldr marked this conversation as resolved
ldr commented 2023-12-25 18:50:53 +01:00
Author
Owner
Copy link

Der Code sieht im allgemeinen gut aus, ich jab ein paar Anmerkungen hinterlassen.
Graphisch sieht es größtenteils auch gut aus,
ich finde den Header jedoch etwas überfüllt.

Das mit dem überfüllten Header stimmt, das werde ich aber erstmal nicht ändern. In der Praxis sind da auch nicht ganz so viele Buttons (Test fehlt), aber da müsste ich alles nochmal in Header und Footer trennen.

> Der Code sieht im allgemeinen gut aus, ich jab ein paar Anmerkungen hinterlassen. > Graphisch sieht es größtenteils auch gut aus, > ich finde den Header jedoch etwas überfüllt. Das mit dem überfüllten Header stimmt, das werde ich aber erstmal nicht ändern. In der Praxis sind da auch nicht ganz so viele Buttons (Test fehlt), aber da müsste ich alles nochmal in Header und Footer trennen.
👍 1 
- Also require webhook_url to be a valid Url
ldr commented 2023-12-25 19:10:46 +01:00
Author
Owner
Copy link

The list of third-party dependencies needs to be rebuilt. I have seen the script, but the file seems to actually be stored in git-lfs at github.com?

The list of third-party dependencies needs to be rebuilt. I have seen the script, but the file seems to actually be stored in git-lfs at github.com?
ben commented 2023-12-25 20:00:14 +01:00
Owner
Copy link

The list of third-party dependencies needs to be rebuilt. I have seen the script, but the file seems to actually be stored in git-lfs at github.com?

They should be in git-lfs here in gitea and the web interface is showing me the file as expected.

> The list of third-party dependencies needs to be rebuilt. I have seen the script, but the file seems to actually be stored in git-lfs at github.com? They should be in git-lfs here in gitea and the web interface is showing me the file as expected.
ben requested review from ben 2023-12-25 20:07:05 +01:00
ben approved these changes 2023-12-25 20:11:58 +01:00
Dismissed
ben left a comment
Copy link

Would be good if the list of third-party dependencies/licenses could be updated as noted, but otherwise this looks good to me 👍

Would be good if the list of third-party dependencies/licenses could be updated as noted, but otherwise this looks good to me 👍
ldr dismissed ben's review 2023-12-25 21:21:12 +01:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

ldr commented 2023-12-25 21:22:30 +01:00
Author
Owner
Copy link

The VSCode UI just displayed it weirdly. 🤷‍♂️

The VSCode UI just displayed it weirdly. 🤷‍♂️
ldr merged commit 009b0fa184 into main 2023-12-25 21:23:53 +01:00
ldr deleted branch facelift 2023-12-25 21:23:53 +01:00
Sign in to join this conversation.
Reviewers
No reviewers
ben
Labels
Clear labels   
bug
Something is not working

  
duplicate
This issue or pull request already exists

  
enhancement
New feature

  
help wanted
Need some help

  
invalid
Something is wrong

  
question
More information is needed

  
wontfix
This won't be fixed

No labels
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
ldr
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
FS-InfMath/Jobboerse!43
No description provided.