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

ECO-124 Captcha na registracie #97

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

Conversation

michal-rohacek
Copy link
Contributor

@michal-rohacek michal-rohacek commented Jul 8, 2021

este to ma nedostatky, pozri to prosimta ako hrubu stavbu, a ked to v principe schvalime, tak dorobim testy a malickosti
(napriklad registration.finish sa mi tiez moc nelubi.. a podobne detaily)

@michal-rohacek michal-rohacek requested a review from jsuchal July 8, 2021 07:01
.env Outdated Show resolved Hide resolved
app/models/registration.rb Outdated Show resolved Hide resolved
app/controllers/registrations_controller.rb Outdated Show resolved Hide resolved
app/controllers/registrations_controller.rb Outdated Show resolved Hide resolved
app/helpers/registrations_helper.rb Outdated Show resolved Hide resolved
app/helpers/registrations_helper.rb Outdated Show resolved Hide resolved
app/models/registration.rb Outdated Show resolved Hide resolved
Comment on lines +11 to +12
RECAPTCHA_SITE_KEY_V3=6LcNQjAbAAAAAPNGbQNxDu0RCKOFOHRdopkJ4bU4
RECAPTCHA_SECRET_KEY_V3=6LcNQjAbAAAAAFvDD1DQJfwWVsF4npB9pI928TpU
Copy link
Member

Choose a reason for hiding this comment

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

Toto by sme nemali davat do public repo a ani do private repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

su to specialne na testing kluce, povoleny maju len localhost..... Prislo mi to praktickejsie, ako predstava, ze ked si budes najblizsie lokalne pustat projekt, budes 15 minut riesit, preco ti tam ukazuje nic nehovoriacu chybu
image
😆
v produkcii sa budu kluce citat z ENV samozrejme.

dame prec?

Copy link
Member

Choose a reason for hiding this comment

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

Tak treba fixnut error hlasku.

.env.test Show resolved Hide resolved
app/lib/environment.rb Outdated Show resolved Hide resolved
app/controllers/registrations_controller.rb Outdated Show resolved Hide resolved
def create
@registration ||= Registration.from(registration_params)

if @registration.save { validate_captcha! }
Copy link
Member

Choose a reason for hiding this comment

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

Co robi ten block?

Copy link
Contributor Author

@michal-rohacek michal-rohacek Jul 26, 2021

Choose a reason for hiding this comment

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

block sa pusti medzi validaciou fieldov (ak je nevalidny email/iny field, ani neposielame captcha request) a medzi finalnym registracnym POSTom

image

Copy link
Member

Choose a reason for hiding this comment

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

Toto je velmi zvlastny pattern, ja by som skor vytiahol von ten block aj ten post do controllera.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nechcem rypat ale
#97 (comment)
... teda, potom je save zbytocne nie?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mozno taketo cosi jedine. ale to asi nesedi na ten tvoj pattern ktory som linkol vyssie
image

Copy link
Member

Choose a reason for hiding this comment

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

No patterny su dva. bud sa to udeje v controlleri alebo v save. Ty si spravil nieco medzi co nebude fungovat ked tam ten block nedas.

Copy link
Member

Choose a reason for hiding this comment

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

Podla mna ten druhy pattern pouzime. Nech sa to deje v controlleri.

app/models/registration.rb Show resolved Hide resolved
app/models/registration.rb Outdated Show resolved Hide resolved
app/views/services/autoform/index.html.erb Show resolved Hide resolved
# redirect all /api request to to datahub.ekosystem
match '/api/(*endpoint)', to: redirect(host: 'datahub.ekosystem.slovensko.digital'), via: :all
# redirect all /api request to proper datahub.ekosystem api
match '/api/(*endpoint)', to: redirect(host: Environment.api_host), via: :all
Copy link
Member

Choose a reason for hiding this comment

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

Rozmyslam, ze na co toto tu vobec mame.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

napadaju ma len historicke dovody alebo zbludile requesty

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.

2 participants