Add contact-info view #312

cekk · 2018-11-11T13:54:56Z

Seems working good.
The only problem that i have, is with the formData.
I need to pass formData={{ tilesLayoutFieldname: {} }} to avoid some errors in the Form component.

…contact_form

coveralls · 2018-11-11T14:11:47Z

Pull Request Test Coverage Report for Build 2730

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at ?%

Totals
Change from base Build 2726:	0.0%
Covered Lines:
Relevant Lines:	0

💛 - Coveralls

tisto · 2018-12-02T07:51:25Z

src/components/manage/Sharing/__snapshots__/Sharing.test.jsx.snap

@@ -80,9 +80,7 @@ exports[`Sharing renders a sharing component 1`] = `
            </th>
            <th
              className=""
-            >


@cekk this seems to be unrelated to the contact-info view (same for the translation). Did you do this by purpose or accident?

Yes, it's unrelated, but i had this broken test and without this change i can't had green lights

tisto · 2018-12-02T07:52:00Z

src/components/theme/ContactInfo/ContactInfo.jsx

@@ -0,0 +1,238 @@
+/**
+ * Login container.


@cekk you forgot to amend the comments here. Should be "ContactInfo" not "Login"

tisto · 2018-12-02T07:53:40Z

src/components/theme/Footer/Footer.jsx

-        <List.Item href="/contact">
-          <FormattedMessage id="Contact" defaultMessage="Contact" />
+        <List.Item>
+          <Link to="contact-info" className="item">


@cekk Maybe this is our chance here to rename "/contact-info" to something saner. This is a contact form, right? I never understood why we use "info" here. Especially since Plone does not allow you to edit the contact info page as an editor.

Yes, you're right.
I had a discussion with @robgietema in Tokyo and we decided to keep "contact-info" for retro-compatibility with the plone environment.
But i'd like to change it in "contact" or "contacts" or whatever.
When we decide the name, i could also fix the typo in the other comment.

P.S.: github didn't emailed me yesterday with this review :(

@sneridagh @robgietema I'd prefer to change "contact-info" to "contact-form" but I won't fight for it. My opinioin is that if we don't change things now in Volto for the better, we never will. :)

contact-form is fine for me also

But then we also need to change the classname etc to make in consistant.

cekk · 2018-12-10T15:31:59Z

@tisto did you received some notifications about my replies on your reviews?
Because i didn't get any email from github for yours.

tisto · 2018-12-10T19:07:28Z

@cekk yep. Got your comments via email...

* master: (99 commits) Back to development Prepare for release Fix build for projects created with Please specify the project directory: create-volto-app <project-directory> For example: create-volto-app my-volto-app Run create-volto-app --help to see all options. Back to development Prepare for release 1.5.0 Add support for extending Semantic UI Styling using the semantic theme engine by adding an file Default tile position to center for all the existing tiles Upgrade api/ to Plone 5.1.3. Update README.md Update README.md Update README.md Fix tests Add guards if user enters bad Embed code, and related info (and error) message Update changelog Add supported browsers disclaimer Fix Header scroll in Firefox in case that there are lot of items in the nav Update yarn.lock. Add changelog and documentation Downgrade query-string Try downgrade again ...

sneridagh · 2019-02-19T18:11:41Z

@cekk Just merged latest master into the branch. Overall it LGTM, but one question, I was considering two things:

Check if the backend has a correct mail setup (Plone already does that)
Add a captcha (like https://github.com/dozoisch/react-google-recaptcha)

I know that the last one might be an opt-in, in case you configure it, but still we can warn integrators about the fact if they do not configure one...

/cc @tisto @robgietema @cekk what do you think?

cekk · 2019-02-19T19:59:53Z

If i remember correctly, i tried it but i couldn't check email config because i needed to call a "private" endpoint: the registry one.
I tried to bypass this thing with a better error handling on plone.restapi.

For captcha field, we need to get the value from registry (from collective.recaptcha), right?

sneridagh · 2019-02-28T13:58:34Z

@cekk having a Plone add-on only to store a string in the registry, then add a service (and a use an XHR call) to retrieve it, and after that validate it (you should also do from the SSR process to not expose the private token), then talk again to the component to give the ok for the code, seems to me like just too much. I don't think that would be that much use cases like this one, but this kind of decisions are the ones that should avoid in the future and we should take it as a strategic policy.

It's true that we can still do the validation on Plone, then add a service for it, but it's that worthy?

/cc @tisto @robgietema @vangheem @bloodbare what do you think?

Probably I can draft something in the next days implementing the solution that I proposed.

cekk · 2019-02-28T16:37:41Z

Ok, as I said you're more into Volto than me, so you better know what's the vision/policy about its features ;)

* master: (86 commits) Back to development Release 1.6.1 Prepare for release Fix regression on position of the add tile button Remove debug dangling logging Fix post release back to development script, by adding semver.inc helper Fix version on back to development manually Back to development Release 1.6.0 Fix imports on false merge with master Add error log in the SSR console. Fix edit forms with richtext fields coming from SSR. Update docs Add changelog Fix images requests that require API auth using a SSR helper Fix not valid <div> tag nested in a <p> tag error on tiles and wysiwyg field Re-add width stmt that was deleted by accident. Set min-width to flating images. Docs polishing, first round Set image width in Volto editor to 50% for images that float left/right. Force use python3 on mkdocs ...

… of the secrets used only in SSR mode for recaptcha feature. Add feature to contact form as first consumer.

sneridagh · 2019-03-02T16:34:22Z

@cekk @vangheem ok, first implementation in place.

I added a new tokens stash where the private ones are not exposed to the client bundle (tested and works well).

I added the recaptcha feature to be generic for all <Form> component and API-less dependent via a prop recaptcha.

However, after having all implemented, I realised that the mail endpoint is still not protected "as must be" (something that we have in mind), and we probably require to implement the recaptcha validation on it directly for this special case. @bloodbare told me that Guillotina provides this OOTB :) The problem is that we are in a point were doing changes in Plone core is difficult (since 5.2 is feature freezed), although for plone.restapi things are a bit different we will have to introduce a few dependencies (at least the recaptcha verification library) which I don't know if the FWT will be up to it...

Anyways, I would go with this implementation and leave the endpoint improvement for some of the upcoming sprints. For now it works for all forms if the prop is enabled, which it already a great feature for me. What do you think?

/cc @tisto @bloodbare @robgietema

tisto · 2019-03-19T13:59:11Z

@sneridagh how would the user add the recaptcha token? Do I need a create-volto-app app to do this? I think we are missing some docs here. Though, maybe we should agree if that's the way to go before putting more effort into it.

@bloodbare @robgietema @vangheem opinions?

vangheem · 2019-03-19T14:04:44Z

woah, https://plonerestapi.readthedocs.io/en/latest/email-send.html isn't protected?

Can we just schedule a team call to discuss these issues? I feel like at this point, we need a longer form discussion to get through everything that is going on related to integration.

sneridagh · 2019-03-19T14:19:24Z

@vangheem it is, because by default only "Manager" role can send mails. We know that it's not the best solution, but for sure it won't be the last implementation. It's on the list of "things to improve".

@bloodbare told me how you do it in Guillotina, I'm +1 for using the same approach and expand it to use it in "all forms". However, not the best moment to introduce changes into the core :) then there's also the inclusion of a third party tool in the core is also to be considered carefully.

@tisto I was waiting to see if that implementation that I did was good enough, and I was waiting for feedback before documenting it. My idea is to use that approach until we have something better in the back. Also, I was -1 in making the config on the Plone side.

vangheem · 2019-03-19T14:29:59Z

Could we schedule a call about this to discuss?

tisto · 2019-03-19T15:39:42Z

Yep. Let's do that.

* master: (130 commits) Fix not valid css-loader options Upgrades update css-loader Add Origin to allow_headers in CORS settings, otherwise Firefox is not allowing CORS Back to development Release 2.1.3 Final cleaning up, polish Makefile run build Polish Fix travis Missing Volto build step Fix traviiis Use xenial everywhere Fix travis Last fixes Back to development Release 2.1.2 Add changelog Fix the folder_contents view component bby preventing the SearchableText be empty if you haven't typed anything in the filter fields yet. This is caused by the new ZCatalog in Zope 4. WIP ...

…entation of the secrets used only in SSR mode for recaptcha feature. Add feature to contact form as first consumer." This reverts commit f79a244.

sneridagh · 2019-04-18T10:36:15Z

@cekk
@robgietema and me had a talk about this yesterday, and some weeks ago I discussed this with @vangheem So we concluded:

We ship this for now in the same shape that Plone currently provides (a non "secure" form) not using any captcha mechanism.
We will investigate and develop a way to do client-server validation, ideally without relying a third party service (Google recaptcha) to do that during the Toulouse and Bonn sprints.

So I would merge it right away.

robgietema · 2019-04-18T11:10:44Z

src/components/theme/Footer/Footer.jsx

-        <List.Item href="/contact">
-          <FormattedMessage id="Contact" defaultMessage="Contact" />
+        <List.Item>
+          <Link to="contact-info" className="item">


contact-form is fine for me also

src/config/NonContentRoutes.jsx

src/components/theme/ContactInfo/ContactInfo.test.jsx

robgietema · 2019-04-18T11:14:04Z

src/components/theme/Footer/Footer.jsx

-        <List.Item href="/contact">
-          <FormattedMessage id="Contact" defaultMessage="Contact" />
+        <List.Item>
+          <Link to="contact-info" className="item">


But then we also need to change the classname etc to make in consistant.

cekk added 17 commits November 11, 2018 09:49

add contactinfo container

62755f4

add contact info to config files

96d77a4

set contact link as a Router Link

61211c4

fix code format

2b9079e

update translations

a988978

update outdated snapshots

3fdf90e

update changelog

d2de837

fix tests

7778086

add contactinfo container

3cb7f00

add contact info to config files

1d8c9e4

set contact link as a Router Link

169b084

fix code format

a14e119

update translations

dffe5c4

update outdated snapshots

7e8f58b

update changelog

81264d8

fix tests

570aa82

Merge branch 'cekk_contact_form' of github.com:plone/volto into cekk_…

47debbc

…contact_form

tisto requested review from sneridagh and robgietema November 12, 2018 09:22

Merge branch 'master' into cekk_contact_form

7d08d3f

tisto reviewed Dec 2, 2018

View reviewed changes

tisto added the 32 needs: review label Dec 2, 2018

sneridagh added 2 commits March 2, 2019 09:25

Add generic recaptcha feature for Form component. Fist implementation…

f79a244

… of the secrets used only in SSR mode for recaptcha feature. Add feature to contact form as first consumer.

sneridagh requested review from vangheem and tisto March 2, 2019 16:34

Login -> ContactInfo in code comments.

22cbfae

sneridagh added 3 commits April 18, 2019 12:22

Revert "Add generic recaptcha feature for Form component. Fist implem…

b8c0afc

…entation of the secrets used only in SSR mode for recaptcha feature. Add feature to contact form as first consumer." This reverts commit f79a244.

Ready to merge

7a24be5

robgietema requested changes Apr 18, 2019

View reviewed changes

cekk added 3 commits April 18, 2019 13:39

fix route path

488800f

remove unused import

86552db

rename contact-info into contact-form

c8b4427

cekk requested a review from robgietema April 18, 2019 14:01

robgietema approved these changes Apr 18, 2019

View reviewed changes

Merge branch 'master' into cekk_contact_form

fc14d8b

sneridagh approved these changes Apr 18, 2019

View reviewed changes

sneridagh merged commit 14d937d into master Apr 18, 2019

sneridagh deleted the cekk_contact_form branch April 18, 2019 15:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add contact-info view #312

Add contact-info view #312

cekk commented Nov 11, 2018

coveralls commented Nov 11, 2018 •

edited

Loading

tisto Dec 2, 2018

cekk Dec 3, 2018

tisto Dec 2, 2018

tisto Dec 2, 2018 •

edited

Loading

cekk Dec 3, 2018

tisto Mar 19, 2019

robgietema Apr 18, 2019

robgietema Apr 18, 2019

cekk commented Dec 10, 2018

tisto commented Dec 10, 2018

sneridagh commented Feb 19, 2019

cekk commented Feb 19, 2019

sneridagh commented Feb 28, 2019 •

edited

Loading

cekk commented Feb 28, 2019

sneridagh commented Mar 2, 2019

tisto commented Mar 19, 2019

vangheem commented Mar 19, 2019

sneridagh commented Mar 19, 2019

vangheem commented Mar 19, 2019

tisto commented Mar 19, 2019

sneridagh commented Apr 18, 2019

robgietema Apr 18, 2019

robgietema Apr 18, 2019

Add contact-info view #312

Add contact-info view #312

Conversation

cekk commented Nov 11, 2018

coveralls commented Nov 11, 2018 • edited Loading

Pull Request Test Coverage Report for Build 2730

💛 - Coveralls

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tisto Dec 2, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cekk commented Dec 10, 2018

tisto commented Dec 10, 2018

sneridagh commented Feb 19, 2019

cekk commented Feb 19, 2019

sneridagh commented Feb 28, 2019 • edited Loading

cekk commented Feb 28, 2019

sneridagh commented Mar 2, 2019

tisto commented Mar 19, 2019

vangheem commented Mar 19, 2019

sneridagh commented Mar 19, 2019

vangheem commented Mar 19, 2019

tisto commented Mar 19, 2019

sneridagh commented Apr 18, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Nov 11, 2018 •

edited

Loading

tisto Dec 2, 2018 •

edited

Loading

sneridagh commented Feb 28, 2019 •

edited

Loading