Update People when they are viewed | PuzzleTime Sync #7
Introduction When clicking on an employee that is displayed on the ptime-employee-dropdown it redirects to the detailed view of this employee. As soon as an employee is clicked a request for this employee is sent to the PTime API and the response data will be saved in the skills DB. Then the freshly fetched data is displayed on the profile of this specific person. This makes sure that the data is up to date at all times.
Creating a person
If a Person doesn't exist yet in the Skills DB, we want to create it. The dropdown will have /people/new?ptime_employee_id={ptime_employee_id} as link. When we navigate to this route, we first fetch this ptime_employee_id from the PTime API, to see if it exists and to get the data. If it doesn't exist, we do nothing. We also do nothing, if there already is a person with this ptime_employee_id in the skills DB. If it exists, we create a new person with the fetched data and set it's ptime_employee_id to the id of the fetched employee.
It would probably be beneficial to implement a loading bar while fetching data.
ToDo
- [x] A request is sent everytime a specific person is visited
- [x] The response is rendered correctly with good timing
- [x] Create new person if it doesn't exist yet
- [x] Loading bar if necessary
Stand 03.07.2024
Ich habe im people controller begonnen die show und new Methoden so umzuschreiben, dass nun der Employee gefetched wird und die Daten upgedatet werden sollten. Es ist jedoch noch nicht ganz ausgereift.
Stand 04.07.2024 Das Updaten von Personendaten funktioniert jetzt einigermassen. Ausserdem habe ich die Logik dazu aus den Controllern entfernt und in eine Klasse ausgelagert. Auch das Erstellen von neuen Personen, wenn diese noch nicht existieren, sollte jetzt einigermassen funktionieren.
Stand 05.07.2024 Ich habe mich heute vor allem auf Testing des features konzentriert, aber auch noch einige Bugs behoben. Das feature sollte auch funktionieren und einige Tests sind geschrieben.
Hinweise: Ich habe feature specs als auch domain specs geschrieben.
Was noch fehlt:
- Soll ein error geraised werden, wenn in
update_person_datadie Person keineptime_employee_idhat? - Einen Test schreiben, der überprüft ob default Personen von der
newaction erstellt wurden wirklich gelöscht werden, wenn die jeweiligeptime_employee_idauf der PuzzleTime API nicht gefunden wird. - Testen ob diese default Personen auch gelöscht werden, sollte das model invalide sein vor dem .save
Notes 29.07.2024
- Rename update_person_data class in ptime namespace and methods in it -> Idea for class
PersonEmployee.
Stand 29.07.2024 Ich habe heute die Logik so umgeschrieben, dass die Person nicht mehr zuerst als dummy angelegt und dann geupdated, sondern direkt mit den richtigen Daten erstellt wird. Ausserdem habe ich die Tests entsprechend der neuen Logik aufgeräumt.
Stand 30.07.2024 Heute habe ich meinen Code mit dem Code für das neue Dropdown gerebased. In diesem Code waren auch einige neue Testhelper vorhanden, mit denen ich meine Tests auch entsprechend umgeschrieben habe. Danach habe ich mich darum gekümmert, dass mein Teil des Codes auch ohne Time Anbindung funktioniert. Ich habe den Code der mit dem Time arbeitet in ein Module ausgelagert, welches nun durch einen Initializer conditionally included werden soll.
ToDo
- [x] Move default languages to model
- [x] Rename ptime_connector initializer to ptime
- [x] Rename ptime_connection to person_controller and put it in the ptime namespace
- [x] Make the initializer work
- [x] Refactor update_person_data method by splitting it into multiple methods
Stand 31.07.2024 Heute hatten wir eine Besprechen mit @mtnstar und haben den vorläufigen Stand angeschaut. Basierend auf dem Feedback habe ich die Änderungen (oben bei ToDo ersichtlich) implementiert. Ausserdem habe ich den Initializer gefixt. So kann man nun über eine ENV Variable kontrollieren, ob die Methoden für die Time-Anbindungen verwendet werden oder nicht.
Stand 02.08.2024
Der Branch ptime_mapper_script enthält sowohl das Skript um die IDs der Personen aus dem PuzzleSkills auf die IDs der Personen aus dem PuzzleTime zu mappen als auch die Funktionalität die aktuellen dropdown Daten direkt aus dem PuzzleTime zu beziehen.
Der verlinkte Branch dieses Tickets baut auf dem oben genannten Branch auf und fügt folgende Funktionen hinzu:
Wenn man auf eine Person navigiert, dann fetcht und updated der Controller die Daten von der PuzzleTime API bevor die Person dem Benutzer angezeigt wird. Wenn eine Person noch nicht existiert, dann kann diese über die Route `/people/new?ptime_employee_id=<ptime_employee_id> erstellt werden. Der Controller fetcht dann die Daten und erstellt daraus die entsprechende Person.
Über eine ENV-Variable namens PTIME_API_AVAILABLE kann entschieden werden, ob der Sync mit dem PuzzleTime verwendet werden soll.
Was noch fehlt:
- [x] Ein Test failed noch. Dieser muss noch gefixt werden.
- [x] Der "Profil anlegen" button muss aus dem UI entfernt werden, wenn das PuzzleTime sync verwendet wird.
- [x] Die "/new" route darf das form bzw. template nicht rendern, wenn das PuzzleTime sync verwendet wird.
- [x] Allgemeine Dokumentation mit Informationen über das PuzzleTime Sync zb. im README.
~~Absolutely everything~~ (outdated)
(24.03.2025)
Well then. Like the title said. Everything about this story but most importantly, how to finally bring it to an end.
What is the PTime sync? The PTime sync is used to map certain attributes that are saved in the PuzzleTime DB into the persons available in Skills. This basically means that the PTime is the source of truth for all the data regarding the person. This also means that the button for editing said data or creating a new person is disabled whilst the sync is active.
How does it work? Simply put, it displays all people from the PuzzleTime in the Skills-DB. When clicking on a person that does not exist, it saves this person into the Skills-DB and fetches all the data from the PuzzleTime. If the person already exists in the DB it only does said second step. So fetching the data from the PuzzleTime and then saving these changes into the Skills-DB.
Can one disable it?
Yes it actually is disabled by default. In the application.rb file is a method called use_ptime_sync which fetches a ENV variable to define if the sync is active or not. When developing locally, the ENV variable is not set by any docker config, so it simply uses the default value which is currently set as false.
What happens when the PTime is unavailable? When the PTime API is unavailable or the sync is disabled, the application simply shows the data saved in the DB again.
How does the mapping from a PTime employee to a PSkills person work?
Basically, we have a rake task named rake ptime:assign which maps Employees from the PTime API to the people in the Skills application. This task creates a attribute named 'ptime_employee_id' on each person that got matched. All the new people already come with such an Id which gets sent with the rest of the PTime data.
Now that we covered the basics, let's talk about what works so far, and what does not quite work yet.
Works : )
- Creating a new person based on the data of the PTime
- Updating a persisted person based on the data of the PTime
- Disabling the sync
- Tests
Still in progress
- The DROPDOWN PROBLEM (insert dramatic music)
- Getting all the data wee need from the PTime API
What needs to be done about these two problems.
The DROPDOWN PROBLEM
Originally the dropdown was designed to send a request to the PTime everytime it's clicked. At some point we realized that the potential traffic of this solution would be way too much. So we implemented a quick fix for this. This fix was implementing a ENV variable called LAST_PTIME_REQUEST which basically tracked when the last PTime request was sent for rendering the dropdown. Yet the whole application was designed to run on live data. Which also means a person will only be created in the Skills-DB as soon as someone clicks on this person. See where im going with this? If a new person gets added to the PTime is will get shown in the dropdown every hour. Yet when no one ever pressed on this person it will never be saved in the Skills-DB. Which means in between these hours this person will never show. Probably replace this logic with a delayed job which fetches all the Data from the PTime API every hour but automatically saves the person if a new one got created. Speak with @Robin481 about this, if the explanation was not understandable enough, apologies. He also knows about this problem so maybe ask him.
PTime API Don't worry, this feature is basically done. Yet since the first time when the PTime API got updated we had to switch a few things up. You can see every change in this PR I created: https://github.com/puzzle/puzzletime/pull/244. Here are he changes in this PR summed up:
- Remove the full_name attribute from the API.
- Add the department_name to the API
- Inside the
employment_rolesattribute add the Level (S1, S2, S3...) of the role as well. - Tests
On this PR the Tests still failed and I have no idea why. Speak with Matthias or Thomas about problems with the PTime and make sure to request their help. As soon as this PR is merged and the new version of the PTime got released make sure to comment in the code I wrote for the role_level in the ´people_employees.rb´ file.
Also make sure to add the mapping for the department_name attribute. There is currently no existing code for this.
Conclusion
Here is everything that needs to be done:
- [ ] Replace the dropdown logic with something more reliable
- [ ] Merge the PTime PR with all the needed attributes
- [ ] Map all the new attributes we receive from the PTime API
- [ ] Make sure the mapping script still works as expected (there is a task called 'rake ptime:evaluate_assign' which does a dry run)
- [ ] Tests ofc
- [ ] Update the documentation if needed?
- [ ] The only that should be left after this would be the disabling of editing the person which is gonna be solved in another story.
Attention To use cron expressions with delayed jobs we also have to install the delayed cron jobs gem.
Todo
- [x] Only show employed employees in dropdown
- [x] Check translations
- [x] Documentation
- [x] Feature specs
- [x] Fix tests
Stand 28.04.2025 Die Funktionalität ist nun komplett implementiert und Tests sind auch geschrieben. Etwas ist noch zu klären: Wenn im nightly job ein entry invalid ist, reicht es dann in der error message die Namen der invaliden Personen anzuzeigen, so wie wir es momentan machen, oder sollten alle error messages pro Namen mit enthalten sein.
ToDo
- [x] Show flash message on request timeout
Stand 06.05.2025 Heute habe ich die conversations von @Kagemaru grösstenteils resolved.
Stand 16.05.2025
Da wir nun, wie in Ticket #889 beschrieben auf einen Ansatz mit mehreren Providern wechseln müssen, habe ich begonnen diesen Ansatz umzusetzen. Dafür habe ich mir überlegt, dass wir die ENV Variablen die wir bisher verwendet haben mit einem festen Wert prefixen. Momentan arbeite ich dabei mit PTIME_PROVIDER_<id>. So kann man dann später durch die verschiedenen Provider iterieren und auch über die id zwischen ihnen unterscheiden.
Info Beim Anbinden der PuzzleTime-Instanz von Puzzle Deutschland ist uns aufgefallen, dass dort niemand sein geburtsdatum erfasst hat. Das Problem ist, dass das Geburtsdatum im Skills required ist. Bei unserer Instanz scheint das Geburtsdatum bei jeder Person eingetragen zu sein, weshalb uns dies vorher nie aufgefallen ist.
Stand 20.05.2025 Man kann nun schon mehrere PuzzleTime Provider anbinden und sowohl das Mapping als auch das Updaten der Daten scheint zu funktionieren. Als nächstes müssen noch die Tests entsprechend angepasst werden und eine Admin View erstellt werden, auf der man Inaktive Personen löschen kann.
Stand 21.05.2025 Heute habe ich die Tests so umgeschrieben, dass sie die neue Implementation mit mehreren Providern testen. Ausserdem habe ich die fehlenden translations hinzugefügt. Zum Schluss habe ich noch begonnen ptime helper specs zu schreiben, um die ptime provider config zu testen.
Wenn das auch getestet ist, fehlt noch die Admin view, die man verwenden kann, um inaktive Personen zu löschen.
Feedback 22.05.2025 Ich habe mich mit @kallies unterhalten und von ihm kommt der Wunsch die Präsenz eines Geburtsdatums bei Skills nicht mehr zu erzwingen. Robin und ich finden das eine gute Idee. Daher bitte so umsetzen.
Fragen
- Warum gibt es eine Firma namens
Ex-Mitarbeiterwenn Profile von ausgetretenen Members gelöscht werden? - Momentan mappe ich den provider identifier auf die Firma. Dies kann jedoch zu einigen edge-cases führen, z.B. wenn jemand eine Firma umbenennt. Ausserdem kann so niemand mehr Werte wie
BewerberoderPartnererhalten. Wäre es besser das Feld Firma auch mit dem PuzzleTime sync bearbeitbar zu machen? - Sollte es möglich Profile zu löschen, wenn der sync aktiv ist?
Stand 23.05.2025 Heute habe ich an der Admin-View gearbeitet, die eine Übersicht über die Ex-Members und Members, die nicht vom sync geupdated werden, gibt und es auch direkt erlaubt diese zu löschen.
Stand 28.05.25 Wir hatten entschieden, dass wir für Personen, die in beiden PuzzleTime Instanzen vorhanden sind, die Daten aus der schweizer Instanz nehmen. Es gibt jedoch nun zwei Personen, die ungenutzte Profile im PuzzleSkills Schweiz haben, die korrekten Daten würden jedoch aus der deutschen Instanz kommen.
Ausserdem: Sollten wir Profile, die zwar als angestellt markiert sind, aber nicht vom Ptime Sync geupdated werden, da sie beim mappen nicht gemapped wurden, in der Suche anzeigen?
Stand 24.06.2025 Ich habe nun die Conversations der PR resolved. Zusätzlich habe ich noch das readme aktualisiert, da ich mich dazu entschieden habe, den PuzzleTime sync per default zu deaktivieren. Dies kann natürlich wieder geändert werden, falls dies nicht so gewünscht ist.