-
Notifications
You must be signed in to change notification settings - Fork 58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Contact organizers form #1390
Contact organizers form #1390
Conversation
locales/translation-de.json
Outdated
@@ -260,6 +260,8 @@ | |||
"groups": { | |||
"contact": "Ansprechpartner", | |||
"contacts": "Ansprechpartner", | |||
"contact_the_organizers": "Kontaktieren", | |||
"contactTheOrganizers_label": "Gäste können Ansprechpartner kontaktieren", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Gäste" finde ich zu unspezifisch. Im Moment sollte es lauten "Softwerkskammer-Mitglieder", oder?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Im finalen Ausbau soll das auch für Gäste möglich sein. Unsere Überlegung war, wir wollten die gleiche Konfiguration weiterverwenden. Und damit auch im jetztigen Zustand gleich klar kommuniziert wird für was das da sein soll damit später keiner Überrascht wir, wenn Mails von Gästen kommen
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Da wir hier gleichzeitig von zwei verschiedenen Dingen sprechen, lass uns das mal trennen:
-
Den Begriff "Gäste" finde ich zu unspezifisch. Ich glaube in Diskussionen haben Andreas und ich immer von "Mitglied" vs. "unangemeldeter Besucher" gesprochen. Klingt das plausibel für Euch?
-
"gleich klar kommuniziert wird" -> Wird es im Moment eben gerade nicht. Ihr sagt, das Feature ist für etwas gut, aber wenn man dann schaut, findet man heraus, dass das nicht stimmt. Das finde ich ziemlich verwirrend... Wie wäre es mit "Softwerkskammer-Mitglieder (und später auch unangemeldete Besucher) können ..."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vorschlag: "Ermögliche, Ansprechpartner anzuschreiben"
Vorteil: 1. Kein "kontaktieren" mehr, 2. Ausbaufähig
Zu Nicole: "Gast" ist ein nicht registrierte Sitebesucher. - Solange das Feature nicht für jeden implementiert ist, ist es y.a.g.n.i. (und ich habe Bedenken anzumelden; es durchbricht das Sicherheitsprinzip, dass E-Mail Adressen nie sichtbar und nur für Mitglieder zur indirekten Kontaktaufnahme dienen)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"es durchbricht das Sicherheitsprinzip": Was genau meinst Du hier? Auch wenn Mitglieder miteinander Kontakt aufnehmen, wird sofort die Mailadresse des Sendenden übermittelt.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hier meine ich, dass es
- Nur registrierten Mitgliedern möglich ist, zu senden. Und
- Sie die Empfängeradresse niemals sehen
- Absenderadresse als reply-to für den "versteckten" Empfänger sichtbar ist.
Das "Konzept" liegt darin, dass es so kommuniziert und bekannt ist. Weiterhin wäre dies ein Einfallstor für Aktionen, die ein nicht-registrierter Sitebesucher vornehmen kann. Aktuell schützen wir quasi alle Aktionen mit "isRegistered"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gäste vs "unangemeldeter Besucher"
Wenn das eure bisherige Sprache ist, mach das für mich Sinn.
"Softwerkskammer-Mitglieder (und später auch unangemeldete Besucher) können ..."
Finde ich gut. Dokumentiert den ist Zustand und gibt genug Informationen was es in der Zukunft bedeuten soll das man das gleiche Flag weiterverwenden kann.
"Ermögliche, Ansprechpartner anzuschreiben"
Um den Punkt von Nicole aufzugreifen, ich kann ja jetzt auch schon Ansprechpartner anschreiben. Nur nicht alle auf einmal.
Weiterhin wäre dies ein Einfallstor für Aktionen, die ein nicht-registrierter Sitebesucher vornehmen kann. Aktuell schützen wir quasi alle Aktionen mit "isRegistered"
Die Idee hinter dem Feature ist schon, das nicht angemeldete Besucher die Ansprechpartner kontaktieren können. Das ist noch nicht implementiert, wollen wir aber noch machen. Die emails der Ansprechpartern bekommt der nicht angemeldete Besucher nie zu sehen. Damit die Ansprechpartner Kontakt aufnehmen kann, muss er eine email Adresse mit angeben, die dann auch wieder nur die Ansprechpartner sehen. Das ganze kann noch ueber ein Captcha abgesichert werden.
Ist das etwas, was wir deine Meinung nach nicht anbieten sollten?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gäste vs "unangemeldeter Besucher"
Wenn das eure bisherige Sprache ist, mach das für mich Sinn.
@signed Andreas hat mich ja zu Recht hier korrigiert; wir verwenden sehr wohl den Begriff "Gast". (Siehe auch rechts oben in der SWK.) Passt also dann für mich.
@leider Zum "Sicherheitsprinzip": Die Punkte 2 und 3 wären ja auch weiterhin so. Punkt 1 würde dahingehend aufgebohrt, dass Ansprechpartner nach vorherigem Opt-In auch von Nichtmitgliedern angeschrieben werden könnten. Das passt soweit für mich. Wo genau hast Du hier noch Bedenken?
@leider Was das "Aktionen auslösen können" angeht: Das Ganze muss natürlich mindestens hinter einem Captcha sein. Wäre das für Dich ausreichend? Wenn nicht: Was müsste für Dich noch gegeben sein?
@signed "Damit die Ansprechpartner Kontakt aufnehmen kann, muss er eine email Adresse mit angeben" -> Es genügt aber nicht, das in den Fließtext zu packen. Dafür muss ein eigenes Feld vorgesehen werden. Denn die versendete Mail braucht auch eine Absenderadresse. Dafür nehmen wir aktuell die Mailadresse des Mitglieds.
locales/translation-de.json
Outdated
@@ -285,15 +288,18 @@ | |||
"name": "Die Adresse, unter der die Gruppe \"@softwerkskammer.org\" erreichbar ist.", | |||
"title": "Anzeigetext innerhalb der Site, ist auch Realname für E-Mails", | |||
"x_coord": "Waagerechte Position auf der Karte", | |||
"y_coord": "Senkrechte Position auf der Karte" | |||
"y_coord": "Senkrechte Position auf der Karte", | |||
"contactTheOrganizers": "Auf der Gruppenseite wir ein Kontaktlink angezeigt, über den Besucher ohne Anmeldung Kontakt mit den Ansprechpartnern aufnehmen können." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Besucher ohne Anmeldung" -> "Softwerkskammer-Mitglieder" ?
"wir" -> "wird" :-)
locales/translation-de.json
Outdated
}, | ||
"type": "Gruppenart", | ||
"x_coord": "X-Koord.", | ||
"y_coord": "Y-Koord." | ||
}, | ||
"mailsender": { | ||
"contact_persons_cannot_be_contacted": "Ansprechpartner können für diese Gruppe nicht kontaktiert werden.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Das ist so nicht richtig, denn man kann sie ja immer individuell kontaktieren. Dafür ist ja das "Ansprechpartner"-Label eigentlich gedacht.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was wir im Moment haben ist ein erster Zwischenschritt. Das Ziel ist schon das nicht angemeldete Besucher (Gäste) die Ansprechpartner kontaktieren können. Dafür fehlt aber noch etwas (siehe description)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ich habe mich hier ausschließlich auf das Wording bezogen. "können nicht kontaktiert werden" stimmt nicht. Weder jetzt noch in Zukunft. (Gäste können sich immer anmelden und dann die Ansprechpartner kontaktieren.) Also bitte etwas umformulieren ;-)
softwerkskammer/lib/groups/group.js
Outdated
@@ -20,6 +20,7 @@ class Group { | |||
this.mapX = object.mapX; | |||
this.mapY = object.mapY; | |||
this.shortName = object.shortName; | |||
this.contactTheOrganizers = object.contactTheOrganizers; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Das klingt mehr wie ein Befehl als ein Flag... Aber mir fällt grad auch nichts Besseres ein, außer dasselbe wie im nächsten Kommentar ("canTheOrganizersBeContacted")
softwerkskammer/lib/groups/index.js
Outdated
@@ -17,7 +18,7 @@ const statusmessage = beans.get('statusmessage'); | |||
const app = misc.expressAppIn(__dirname); | |||
|
|||
function groupSubmitted(req, res, next) { | |||
const group = new Group(req.body); | |||
const group = bodyToGroup(req.body); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ich verstehe nicht, warum Du das gemacht hast... kannst Du mir das erklären? Das Übersetzen des Parameters wäre doch im Group-Konstruktor sehr gut aufgehoben.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ich mag es wenn meine Konstruktoren dumm sind, Felder zuweisen fertig.
Und in dem Fall finde ich es nicht gut das Details des Auslieferungs Mechanismus Web Form Input in die Gruppe zu packen. Die Logik wir auch nicht gebraucht, wenn man die Gruppe wieder aus der Datenbank ließt. Ich habe dafür gern eigene dedizierte Fabrikmethoden.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ich mag es wenn mein Code gleichförmig ist ;-)
Wie Andreas schon hingewiesen hat, wäre die geeignete Vorlage die Methode fillFromUI aus der Member-Klasse.
Davon abgesehen wäre das alles überhaupt kein Thema, wenn Du die UI schon so designen würdest, dass sie gleich den richtigen Wert schickt. Dann kannst Du die ganze Übersetzungslogik sparen. Das wäre mir die allerliebste Variante.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wenn Du die UI schon so designen würdest, dass sie gleich den richtigen Wert schickt.
Ich freue mich über Vorschläge. Meine Erfahrung mit Forms im Web ist ziemlich begrenzt.
@@ -30,7 +30,12 @@ block content | |||
h1 #{group.longName} #{' '} | |||
small #{group.type} | |||
p | |||
strong #{t('general.address')}: #{' '} | |||
if (group.contactTheOrganizers && accessrights.canParticipateInGroup(group)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alle derartigen Berechtigungs-Entscheidungen sollten in den Accessrights gesammelt werden, damit sie leicht zentral änderbar sind.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Das eine ist eine Gruppeneigenschaft, das andere Berechtigung
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was meinst Du damit? Die Logik, dass diese Gruppeneigenschaft so vorliegen muss, gehört doch zur Berechtigung dazu. Dieses Wissen hätte ich gern an einer Stelle in den accessrights.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ach so. Für mich ist das eine ein Anzeigeproperty, wohingegen Sicherheitssachen insbesondere Eigenschaften des Users prüfen
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, verstehe, ok. Dann würde ich dafür plädieren, dass es zumindest eine eigene accessrights-Methode für den Berechtigungsteil des Ausdrucks gäbe. Denn die Logik soll sich ja dann mal ändern. Das fände ich schon nicht schlecht, wenn die dann gleich zentral wäre.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mit "Berechtigungsteil des Ausdrucks" ist dann goup.contactTheOrganizers
(bzw. ein besserer Name, wenn wir einen gefunden haben ;-) ) gemeint?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ich habe jetzt mal den gesamten Ausdruck in die neue Methode canContactTheOrganizers
in accessrights.js
gepackt. Erschien mir recht stimmig, dass zusammenzufassen. Ich kann das aber auch wieder auseinander ziehen.
Sehr cool, danke! Ich beschränke mich mal auf UI-Kritik (auf Deutsch): Das ist zu unauffällig. -> Vorschlag: fetter grüner A-Mail-Button mit "E-Mail an die Ansprechpartner" "kontaktieren": Der Duden kennt dies zwar (https://www.duden.de/rechtschreibung/kontaktieren) allerdings dreht sich mir bei dem Wort immer noch der Magen rum. Bitte vermeiden. Lokal getestet, manuell: E-Mail kommt an, allerdings erkenne ich als Ansprechpartner nicht, dass es eine entsprechende Anfrage ist. Der Absender steht als klare E-Mail Adresse im Reply-To. Vorschlag: Subject präfixen: [Anfrage an Gruppenansprechpartner] (oder ähnlich) |
@@ -0,0 +1,13 @@ | |||
const conf = require('simple-configure'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bitte hier analog Nicoles Vorschlag das ganze in den Group-Konstruktor inlinen. Beispiel aus member.js fillFromUI:
['notifyOnWikiChanges'].forEach(property => {
this.state[property] = !!object[property];
});
btw.: wenn body.contactTheOrganizers nicht 'on' ist, ist es insbesondere auch nicht vorhanden!
@@ -30,7 +30,12 @@ block content | |||
h1 #{group.longName} #{' '} | |||
small #{group.type} | |||
p | |||
strong #{t('general.address')}: #{' '} | |||
if (group.contactTheOrganizers && accessrights.canParticipateInGroup(group)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Das eine ist eine Gruppeneigenschaft, das andere Berechtigung
softwerkskammer/lib/groupsAndMembers/groupsAndMembersService.js
Outdated
Show resolved
Hide resolved
softwerkskammer/lib/groupsAndMembers/groupsAndMembersService.js
Outdated
Show resolved
Hide resolved
@@ -37,11 +37,13 @@ function messageSubmitted(req, res, next) { | |||
if (req.body.nickname) { | |||
return mailsenderService.sendMailToMember(req.body.nickname, message, processResult); | |||
} | |||
if (req.body.contactPersonsGroupName) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
komischer Name. Besser wäre groupname oder ähnlich (groupNameForContact)
groupsService.getGroups([groupId], (groupLoadErr, groups) => { | ||
if (groupLoadErr) { return callback(groupLoadErr, mailtransport.statusmessageForError(type, groupLoadErr)); } | ||
if (groups.length !== 1) { | ||
const groupNotFoundError = new Error(`${groups.length} Gruppen für Id ${groupId} gefunden. Erwarte genau eine Gruppe.`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was hilft das dem Anwender? - Bitte ändern in etwas actionables wie "Das senden der E-Mail ist fehlgeschlagen. Es liegt ein technisches Problem vor."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Eventuell gemeinsam mit nächstem Error behandeln.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Und dem nächsten. Hier wäre allerdings noch besser, dann den Dialog nicht zu ermöglichen! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wenn das feature in der Gruppe nicht aktiviert ist wir der Link auf der Gruppenseite nicht angezeigt.
Die Überprüfung würde ich aber trotzdem drin lassen, für alle die über curl an der Schnittstelle rumspielen wollen.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wie meinst Du das mit dem Rumspielen? - Die Prüfung, ob man senden darf, ist davon ja nicht berührt. Mir geht es um das finden einer Gruppe nach ID.
Also:
- Gruppe suchen -> ggf. techn Fehler
- Gruppe auf feature prüfen / keine Organizer -> mailsender.contact_persons_cannot_be_contacted
Insbesondere ist ja richtig, dass wenn kein Organizer, dann kann auch nicht gesendet werden! ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@leider @signed und mein Punkt bzgl. "mit curl rumspielen" ist, dass der Dialog zwar nicht verlinkt ist, wenn das Feature deaktiviert ist, aber man trotzdem auf die URL einen POST ausführen kann (ich glaube sogar, wenn man die URL für den Dialog kennt, kann man ihn auch aufrufen, wenn das Feature deaktiviert ist). Daher wollten wir das hier nochmal prüfen. Ein Szenario bei dem das sogar relevant wäre:
- Feature ist gerade aktiviert
- Nutzer ruft Formular auf
- es vergeht etwas Zeit, z. B. Nutzer klappt Notebook zu
- Feature wird deaktiviert
- Nutzer klappt Notebook z. B. am nächsten Tag wieder auf und nutzt den Dialog zum versendne
- Mail wird aber nicht versendet, da wir das nochmal abprüfen
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Durch die Anpassung des Fehlertexts in 3a301c8 war das Zusammenfassen der beiden Fehler nicht mehr so einfach möglich. Dafür wird jetzt dem Nutzer bei keiner oder mehreren gefunden Gruppen für eine Id jetzt nur noch "Das senden der E-Mail ist fehlgeschlagen. Es liegt ein technisches Problem vor." angezeigt und die Details geloggt.
locales/translation-de.json
Outdated
"mail_the_organizers": "E-Mail an die Ansprechpartner", | ||
"contactTheOrganizers_label": "Gäste können Ansprechpartner kontaktieren", | ||
"mail_the_contact_persons": "E-Mail an die Ansprechpartner", | ||
"mail_the_contact_persons_label": "Softwerkskammer-Mitglieder (und später auch Gäste) können eine E-Mail an die Ansprechpartner senden.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Für ein Label viel zu viel Text (deswegen gibt es ja den hint). Vorschlag: "E-Mail an Ansprechpartner möglich" - Im Englischen dann natürlich entsprechend.
Nach wie vor gilt: "Doppelt ist redundant. - Doppelt ist redundant."
if (group.contactTheOrganizers && accessrights.canParticipateInGroup(group)) | ||
p | ||
a.btn.btn-success(href='/mailsender/contactGroupContactPersons/' + group.id) | ||
| #{t('groups.mail_the_organizers')} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Der Button ist cool.
Mir gefällt es gut. @NicoleRauch was meinst Du? |
Also abgesehen von den kleinen noch offenen Dingen von @signed ;) |
Fällt mir grad schwer zu beurteilen, ehrlich gesagt - es gab so viele Diskussionen, aber mir ist noch nicht ganz klar, was jetzt noch wann wie geändert wird. Ich bin auch noch nicht durch mit dem Review. Das liegt daran, dass ich mir einen Review-Prozess etwa so vorstelle:
Wenn ich in Step 2) "genug" Punkte gefunden habe, dann höre ich meistens erstmal auf und warte auf die erste Iteration dieses Prozesses, um weiterzuschauen. Das liegt hauptsächlich daran, dass ich nicht ständig in Codestellen reinlaufen möchte, deren Problem ich schon beschrieben habe. Aktuell ist mein Stand, dass Ihr in Step 3) drin seid, aber ich glaube nicht, dass der schon abgeschlossen ist. (Korrigiert mich, wenn ich falsch liege.) |
Du hast recht. Mein Kommentar war übereilt euphorisch.
… Am 28.04.2019 um 10:24 schrieb Nicole Rauch ***@***.***>:
Fällt mir grad schwer zu beurteilen, ehrlich gesagt - es gab so viele Diskussionen, aber mir ist noch nicht ganz klar, was jetzt noch wann wie geändert wird.
Ich bin auch noch nicht durch mit dem Review. Das liegt daran, dass ich mir einen Review-Prozess etwa so vorstelle:
Jemand stellt einen PR (Person X)
Ich oder andere geben gewisse Anregungen ("dies könnte so-und-so besser sein" etc.)
Person X reagiert:
Person X diskutiert eine Anregung und überzeugt mich vom Gegenteil. Dann kann meine Anregung ignoriert werden.
Person X setzt die Anregung um, und zwar nicht nur an der vom Kommentierenden hervorgehobenen Stelle, sondern überall da, wo es relevant ist. (!)
Sind alle Anregungen dementsprechend behandelt, schaue ich nochmal drüber und gebe ggf. weitere Anregungen.
Wenn ich in Step 2) "genug" Punkte gefunden habe, dann höre ich meistens erstmal auf und warte auf die erste Iteration dieses Prozesses, um weiterzuschauen. Das liegt hauptsächlich daran, dass ich nicht ständig in Codestellen reinlaufen möchte, deren Problem ich schon beschrieben habe.
Aktuell ist mein Stand, dass Ihr in Step 3) drin seid, aber ich glaube nicht, dass der schon abgeschlossen ist. (Korrigiert mich, wenn ich falsch liege.)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
@NicoleRauch sieht das richtig: @signed und ich sind noch in der Phase die Anmerkungen umzusetzen (sieht man auch daran, dass noch nicht alle TODOs im ersten Kommentar abgehakt sind). Wenn wir mit allem durch sind würden wir uns nochmal melden, damit der nächste Review-Lauf starten kann :-). |
@NicoleRauch gibt es aktuell Stellen bei der wir die Anmerkungen nur teilweise umgesetzt haben oder war das eher eine allgemeine Aussage wie du dir einen PR-Review-Prozess vorstellst? |
@UrsMetz Das war eine allgemeine Beschreibung, wie ich mir den Review-Prozess vorstelle. Wenn ich in einer Folgeiteration feststelle, dass Anmerkungen nur teilweise umgesetzt wurden, werde ich sie wieder anmerken. Ich möchte nur vermeiden, im Laufe des Reviews mehrfach auf dasselbe zu stoßen und dann unabsichtlich doppelt zu kommentieren. |
85be168
to
70b122b
Compare
@NicoleRauch @leider Jetzt sollten alle bisher im Review aufgefallenen Punkte adressiert worden sein (oder weg diskutiert ;-) ). Der Branch ist auch auf den aktuellen master-Stand rebased. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Das wird doch! - Jetzt isses nur noch Luxuskritik
softwerkskammer/lib/groupsAndMembers/groupsAndMembersService.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ich habe den Großteil der Sachen aus @leider's Review angepasst, bis auf die Sache const
vs. function
in accessrights_test.js
weil ich vorher nochmal geklärt haben wollte was jetzt die Politik bzgl. dieser Sache ist, s. meinen Kommentar dort.
…eachs in it blocks
cde9535
to
5d21134
Compare
Jetzt sollten alle Anmerkungen adressiert sein. |
@@ -285,15 +287,18 @@ | |||
"name": "Die Adresse, unter der die Gruppe \"@softwerkskammer.org\" erreichbar ist.", | |||
"title": "Anzeigetext innerhalb der Site, ist auch Realname für E-Mails", | |||
"x_coord": "Waagerechte Position auf der Karte", | |||
"y_coord": "Senkrechte Position auf der Karte" | |||
"y_coord": "Senkrechte Position auf der Karte", | |||
"mail_the_contact_persons": "Auf der Gruppenseite wird ein E-Mail-Knopf angezeigt, über den die Ansprechpartnern angeschrieben werden können." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ansprechpartner
@@ -309,6 +314,7 @@ | |||
"send": "Senden", | |||
"subject": "Betreff", | |||
"to_member": "An Mitglied", | |||
"to_contact_persons": "An Ansprechpartner der Gruppe", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An die Ansprechpartner der Gruppe ?
@@ -291,15 +293,18 @@ | |||
"name": "The e-mail address \"@softwerkskammer.org\" where the group can be reached.", | |||
"title": "Name of the group used in the website, also realname for e-mails sent by the group", | |||
"x_coord": "Horizontal position on the map", | |||
"y_coord": "Vertical position on the map" | |||
"y_coord": "Vertical position on the map", | |||
"mail_the_contact_persons": "An e-mail button is displayed on the group page, which can be used to write to the contact persons." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ich halte die Grammatik für etwas zweifelhaft... "... on the group page. It ..." halte ich für wesentlich glaubwürdiger.
This adds a form to contact the organizers of a group. The idea is that often people just want to ask a question or propose an idea to the organizers of a group but do not want to contact the whole group via the mailing list.
For now only for signed in users. Enabling this for guest (which was our idea in the first place) would need:
TODOs aus Review:
mailsender.contact_persons_cannot_be_contacted
präziser formulierengroup_has_no_organizers
entfernen?: Muss noch drin bleiben, s. Es kann Gruppen ohne Ansprechpartner geben #1391Group.isContactTheOrganizersEnabled
inGroup.canTheOrganizersBeContacted
fillFromUI
sendMailToContactPersonsOfGroup
überarbeiten, s. Contact organizers form #1390 (comment)getOrganizersOfGroup: function (groupId, callback) {
->getOrganizersOfGroup: function getOrganizersOfGroup (groupId, callback) {
contactPersonsGroupName
ingroupname
oder ähnlich (groupNameForContact
)accessrights
-Methode für "Berechtigungsteil des Ausdrucks", s. Contact organizers form #1390 (comment)to_contact_persons_leave_contact_information_hint
entfernen / Feld für Mailadresse (eigentlich später, wenn Gäste überhaupt das Feature nutzen können)