Skip to content
This repository has been archived by the owner on Feb 5, 2024. It is now read-only.

Frontend: emails opmaken en versturen #423

Merged
merged 21 commits into from
May 19, 2023
Merged

Conversation

arvheule
Copy link
Contributor

@arvheule arvheule commented May 16, 2023

Beschrijving

De klant had een effectieve manier nodig om emails op te maken en te versturen, hier dienen deze aanpassingen in onze code voor.

Motivatie en context

Issue #295 was hiervan de motivatie. De klant is nu in staat een eigen template te definieren alsook een mail te versturen vertrekkende vanuit de gebouwenpagina.

Ook is er een nieuwe administratietabel, waar men de email templates kan oplijsten, bewerken en verwijderen.

Momenteel heb ik een veld weggelaten in de contactform die niet gebruikt werd. De motivatie hiervoor staat vernoemd in #433.

Testmethode

Momenteel zijn hier nog geen testen voor

Screenshots (indien van toepassing):

Screenshot from 2023-05-16 05-49-54
Template aanmaken
Screenshot from 2023-05-16 05-45-13
Email zelf opstellen
Screenshot from 2023-05-16 05-45-29
Administratietabel
Screenshot from 2023-05-16 17-23-34

Aanpassingen

Extra pagina toegevoegd waar men een template zelf kan aanmaken.

  • New feature (non-breaking wijziging met nieuwe functionaliteit)

Checklist

  • De code volgt de stijl en guidelines van dit project.

@arvheule arvheule marked this pull request as draft May 16, 2023 03:51
@codecov
Copy link

codecov bot commented May 16, 2023

Codecov Report

Merging #423 (0e9f3c9) into develop (c700c15) will not change coverage.
The diff coverage is n/a.

❗ Current head 0e9f3c9 differs from pull request most recent head a6ded7d. Consider uploading reports for the commit a6ded7d to get more accurate results

@@           Coverage Diff            @@
##           develop     #423   +/-   ##
========================================
  Coverage    88.55%   88.55%           
========================================
  Files           41       41           
  Lines         1153     1153           
  Branches       237      237           
========================================
  Hits          1021     1021           
  Misses         116      116           
  Partials        16       16           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@arvheule arvheule marked this pull request as ready for review May 16, 2023 15:24
@arvheule arvheule self-assigned this May 16, 2023
Copy link
Contributor

@brmatthy brmatthy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dit ziet er heel goed uit, ik heb enkel nog een paar opmerkingen over de adminiatrator tabel.

  • Vervang mdi-close door mdi-delete als verwijder icoontje
  • Ik zou een warning popup laten verschijnen als je een sjabloon verwijderd, je kan de CardPopup component hiervoor gebruiken.
  • Als je op een rij klikt zou ik ook laten doorlinken naar de template bewerk pagina. net zoals je doet bij het potloodje. Bij de andere admin tabellen is dit ook zo. (eventueel kan het potloodje dan weg gelaten worden, maakt mij niet zo veel uit).

@arvheule
Copy link
Contributor Author

  • Vervang mdi-close door mdi-delete als verwijder icoontje

Gebeurd, goeie opmerking!

  • Ik zou een warning popup laten verschijnen als je een sjabloon verwijderd, je kan de CardPopup component hiervoor gebruiken.

Dit is niet zo makkelijk aangezien deleten de generieke table type gebruikt en deze geen component inherent kan oproepen.

  • Als je op een rij klikt zou ik ook laten doorlinken naar de template bewerk pagina. net zoals je doet bij het potloodje. Bij de andere admin tabellen is dit ook zo. (eventueel kan het potloodje dan weg gelaten worden, maakt mij niet zo veel uit).

De generieke table klasse laat het niet toe om zowel te routen als functionele routes te hebben op een entry. Hierdoor kan je bv verwijderen maar wordt je erna naar het aanpassingscherm gebracht van de ronde die je net verwijderd hebt. Niet routen bij het klikken op een template zelf lijkt me hier dan een betere oplossing.

@arvheule arvheule requested a review from brmatthy May 17, 2023 21:08
brmatthy
brmatthy previously approved these changes May 18, 2023
Copy link
Contributor

@brmatthy brmatthy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oke danku voor de toelichting.

@ludverst
Copy link
Contributor

Ziet er goed uit. de parser werkt niet bij mij.
image

@jenspots
Copy link
Contributor

@matsvbelle, @ludverst wanneer reviewen jullie deze PR?

@jenspots jenspots removed their request for review May 19, 2023 14:01
ludverst
ludverst previously approved these changes May 19, 2023
Copy link
Contributor

@ludverst ludverst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ziet er goed uit

@ludverst ludverst self-requested a review May 19, 2023 15:48
Copy link
Contributor

@ludverst ludverst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

@ludverst ludverst merged commit f460d72 into develop May 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants