Skip to content
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

Crm token migration #224

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Crm token migration #224

wants to merge 15 commits into from

Conversation

Kagemaru
Copy link
Member

No description provided.

Copy link
Member Author

@Kagemaru Kagemaru left a comment

Choose a reason for hiding this comment

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

Hey @svenwey
Deine Lösung sieht eigentlich gut aus. Es ist nicht ganz idiomatischer Ruby/Rails Code, aber ansonsten eine gute Lösung.

Eine Korrektur zu den CSVs hätte ich noch:
Momentan sieht es aus, als ob du von einem einzelnen CSV ausgehst, welches alle Mappings hat. Da die IDs auf Seiten CRM aber aus verschiedenen Models kommt, müssen auch die CSVs separat sein, da es ID Dopplungen auf beiden Seiten (Highrise und/oder Odoo) geben kann.

Hier ein Beispiel Source Folder:
sources.tar.gz

@svenwey
Copy link
Collaborator

svenwey commented Feb 13, 2025

Hey @Kagemaru
Ich habe die Changes an den einzelnen Models auseinandergenommen und prompte nun für jedes Model für ein .csv. Passt das so in der Form einer interaktiven shell-Eingabe der .csv filepaths für dich, oder soll ich diese Pfade definieren und hardcoden?

@Kagemaru
Copy link
Member Author

Kagemaru commented Feb 13, 2025

Ich wäre für eine ENV variable für den source Ordner und hardcoden der CSV-Namen als model.name.underscore da prompts auf den Container Terminals manchmal etwas komisch agieren. Da diese ENV variable nur im Task existiert kann es auch einfach SOURCE sein. rails db:migrate STEP=2 verwendet diese notation.

@svenwey
Copy link
Collaborator

svenwey commented Feb 13, 2025

Ich habe das jetzt mal so umgesetzt. Momentan habe ich noch drin, dass die .csv keine header line haben, das musst du aber sagen, wie du das haben möchtest.

@Kagemaru
Copy link
Member Author

Das können wir machen, wie wir wollen.
Bei dem CSV, welches wir von Dänu bekommen werden, wird es wahrscheinlich einen Header haben.
Das können wir aber manuell anpassen.

kronn and others added 10 commits February 17, 2025 13:25
The first line of the description is shown in the list.
The complete description with 'rake -D'.
Also, this outputs what is being done, providing helpful output.
namespaces in Rake are purely for organizing the tasks, they do not
open a new lexical scope. Therefore, all methods defined here are
bound to Kernel (the implicit receiver). This makes them available
to ALL rake-task and the application if the rake-tasks are being
loaded. It is better to wrap them in a module and call them there.
To avoid further shenanigans, 'module_function' allows calling them
as if they were class-methods.
And since 'warn(msg)' is short for 'stderr.puts', we can simplify and streamline things.
And it does so, without intermediate data-shuffling.

The `model.pluck()` is still needed to get an (indexed) array, so this
cannot be lazy_loaded with some hackery in the SQL-Query. Interesting as
this may be, this is as far as I would optimize it.
While every if could be rewritten to a postfix version, it reads best
for such guard-clauses. The postfix if applies to the line only, so with
  multiline-bodies it is not a viable option.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants