You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2020/09/01 15:21:47 UTC
[GitHub] [couchdb-fauxton] hulkoba opened a new pull request #1292: Feat/news security info
hulkoba opened a new pull request #1292:
URL: https://github.com/apache/couchdb-fauxton/pull/1292
<!-- Thank you for your contribution!
Please file this form by replacing the Markdown comments
with your text. If a section needs no action - remove it.
Also remember, that CouchDB uses the Review-Then-Commit (RTC) model
of code collaboration. Positive feedback is represented +1 from committers
and negative is a -1. The -1 also means veto, and needs to be addressed
to proceed. Once there are no objections, the PR can be merged by a
CouchDB committer.
See: http://couchdb.apache.org/bylaws.html#decisions for more info. -->
## Overview
<!-- Please give a short brief for the pull request,
what problem it solves or how it makes things better. -->
News section shows a 'Load news' button with the information that the IP address is shared with blog.couchdb.org when News are loaded.
Also add the ability to save the choice as a cookie, so the news are loaded without confirmation next time
## Testing recommendations
<!-- Describe how we can test your changes.
Does it provides any behaviour that the end users
could notice? -->
Go on the news-page, click around and check the cookies
## GitHub issue number
<!-- If this is a significant change, please file a separate issue at:
https://github.com/apache/couchdb-fauxton/issues
and include the number here and in commit message(s) using
syntax like "Fixes #472" or "Fixes apache/couchdb#472". -->
#1284
## Related Pull Requests
<!-- If your changes affects multiple components in different
repositories please put links to those pull requests here. -->
This pr is a 'security extension' of https://github.com/apache/couchdb-fauxton/pull/1284
## Checklist
- [x] Code is written and works correctly;
- [ ] Changes are covered by tests;
- [ ] Documentation reflects the changes;
- [ ] Update [rebar.config.script](https://github.com/apache/couchdb/blob/master/rebar.config.script) with the correct tag once a new Fauxton release is made
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-fauxton] hulkoba commented on a change in pull request #1292: Feat/news security info
Posted by GitBox <gi...@apache.org>.
hulkoba commented on a change in pull request #1292:
URL: https://github.com/apache/couchdb-fauxton/pull/1292#discussion_r484372163
##########
File path: app/addons/news/components.js
##########
@@ -9,17 +9,81 @@
// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
// License for the specific language governing permissions and limitations under
// the License.
+import app from "../../app";
import React from "react";
-const NewsPage = () => {
+const LoadNewsButton = ({ showNews, isChecked, toggleCookieSave }) => {
return (
- <div id="news-page" className="">
- <iframe src="https://blog.couchdb.org" width="100%" height="100%"></iframe>
+ <div>
+ <p>
+ When you click this button, you are requesting content and sharing your IP address with <a href="https://blog.couchdb.org/">blog.couchdb.org</a> which is edited by the Apache CouchDB PMC and maintained by <a href="https://wordpress.com/">wordpress.com</a>.
+ </p>
+ <p>
+ If you don’t want to share your IP address, do not click the button.
+ </p>
+ <button className="btn btn-primary" onClick={showNews}>Load News</button>
+ <label className="news-checkbox">
+ <input type="checkbox"
+ checked={isChecked}
+ onChange={toggleCookieSave}
+ />
+ Remember my choice
+ </label>
</div>
);
};
+class NewsPage extends React.Component {
+ constructor (props) {
+ super(props);
+ this.showNews = this.showNews.bind(this);
+ this.toggleCookieSave = this.toggleCookieSave.bind(this);
+
+ const hasCookie = !!app.utils.localStorageGet('allow-IP-sharing');
+
+ this.state = {
+ showNews: hasCookie ? true : false,
+ hasCookie
+ };
+ }
+
+ showNews() {
+ this.setState({ showNews: true });
+ }
+
+ toggleCookieSave() {
+ if (!this.state.hasCookie) {
+ this.setState(() => {
+ return { hasCookie: true };
+ });
+ app.utils.localStorageSet('allow-IP-sharing', true);
+
+ } else {
+ this.setState(() => {
+ return { hasCookie: false };
+ });
+ app.utils.localStorageSet('allow-IP-sharing', false);
+ }
+ }
+
+ render() {
+ return (
+ <div id="news-page" className="">
+ {this.state.showNews ?
Review comment:
@dottorblaster I'm also not really comfortable with that, because the button is not 'news'. I changed it back and save the iframe in the news variable. What do you think?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-fauxton] garrensmith commented on a change in pull request #1292: Feat/news security info
Posted by GitBox <gi...@apache.org>.
garrensmith commented on a change in pull request #1292:
URL: https://github.com/apache/couchdb-fauxton/pull/1292#discussion_r484965642
##########
File path: app/addons/news/components.js
##########
@@ -9,17 +9,73 @@
// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
// License for the specific language governing permissions and limitations under
// the License.
+import app from "../../app";
import React from "react";
-const NewsPage = () => {
+const LoadNewsButton = ({ showNews, isChecked, toggleChange }) => {
return (
- <div id="news-page" className="">
- <iframe src="https://blog.couchdb.org" width="100%" height="100%"></iframe>
+ <div>
+ <p>
+ When you click this button, you are requesting content and sharing your IP address with <a href="https://blog.couchdb.org/">blog.couchdb.org</a> which is edited by the Apache CouchDB PMC and maintained by <a href="https://wordpress.com/">wordpress.com</a>.
+ </p>
+ <p>
+ If you don’t want to share your IP address, do not click the button.
+ </p>
+ <button className="btn btn-primary" onClick={showNews}>Load News</button>
+ <label className="news-checkbox">
+ <input type="checkbox"
+ checked={isChecked}
+ onChange={toggleChange}
+ />
+ Remember my choice
+ </label>
</div>
);
};
+class NewsPage extends React.Component {
+ constructor (props) {
+ super(props);
+ this.showNews = this.showNews.bind(this);
+ this.toggleChange = this.toggleChange.bind(this);
+
+ const allowsIpSharing = !!app.utils.localStorageGet('allow-IP-sharing');
+
+ this.state = {
+ showNews: allowsIpSharing ? true : false,
+ allowsIpSharing
+ };
+ }
+
+ showNews() {
+ this.setState({ showNews: true });
+ }
+
+ toggleChange() {
+ const allowsIpSharing = this.state.allowsIpSharing;
+ this.setState(() => ({ allowsIpSharing: !allowsIpSharing }));
Review comment:
Can you make this `this.setState({ allowsIpSharing: !allowsIpSharing })` and then this is ready to merge. Nice work.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-fauxton] garrensmith commented on a change in pull request #1292: Feat/news security info
Posted by GitBox <gi...@apache.org>.
garrensmith commented on a change in pull request #1292:
URL: https://github.com/apache/couchdb-fauxton/pull/1292#discussion_r482088210
##########
File path: app/addons/news/components.js
##########
@@ -9,17 +9,81 @@
// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
// License for the specific language governing permissions and limitations under
// the License.
+import app from "../../app";
import React from "react";
-const NewsPage = () => {
+const LoadNewsButton = ({ showNews, isChecked, toggleCookieSave }) => {
return (
- <div id="news-page" className="">
- <iframe src="https://blog.couchdb.org" width="100%" height="100%"></iframe>
+ <div>
+ <p>
+ When you click this button, you are requesting content and sharing your IP address with <a href="https://blog.couchdb.org/">blog.couchdb.org</a> which is edited by the Apache CouchDB PMC and maintained by <a href="https://wordpress.com/">wordpress.com</a>.
+ </p>
+ <p>
+ If you don’t want to share your IP address, do not click the button.
+ </p>
+ <button className="btn btn-primary" onClick={showNews}>Load News</button>
+ <label className="news-checkbox">
+ <input type="checkbox"
+ checked={isChecked}
+ onChange={toggleCookieSave}
+ />
+ Remember my choice
+ </label>
</div>
);
};
+class NewsPage extends React.Component {
+ constructor (props) {
+ super(props);
+ this.showNews = this.showNews.bind(this);
+ this.toggleCookieSave = this.toggleCookieSave.bind(this);
+
+ const hasCookie = !!app.utils.localStorageGet('allow-IP-sharing');
+
+ this.state = {
+ showNews: hasCookie ? true : false,
+ hasCookie
+ };
+ }
+
+ showNews() {
+ this.setState({ showNews: true });
+ }
+
+ toggleCookieSave() {
+ if (!this.state.hasCookie) {
+ this.setState(() => {
+ return { hasCookie: true };
+ });
+ app.utils.localStorageSet('allow-IP-sharing', true);
+
+ } else {
+ this.setState(() => {
+ return { hasCookie: false };
+ });
+ app.utils.localStorageSet('allow-IP-sharing', false);
+ }
+ }
+
+ render() {
+ return (
+ <div id="news-page" className="">
+ {this.state.showNews ?
Review comment:
Could you make this an if statement before the jsx code. Something like:
```
let news = <LoadButton />;
if (this.state.hasCookie) {
news = <iframe/>
}
```
```
##########
File path: app/addons/news/components.js
##########
@@ -9,17 +9,81 @@
// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
// License for the specific language governing permissions and limitations under
// the License.
+import app from "../../app";
import React from "react";
-const NewsPage = () => {
+const LoadNewsButton = ({ showNews, isChecked, toggleCookieSave }) => {
return (
- <div id="news-page" className="">
- <iframe src="https://blog.couchdb.org" width="100%" height="100%"></iframe>
+ <div>
+ <p>
+ When you click this button, you are requesting content and sharing your IP address with <a href="https://blog.couchdb.org/">blog.couchdb.org</a> which is edited by the Apache CouchDB PMC and maintained by <a href="https://wordpress.com/">wordpress.com</a>.
+ </p>
+ <p>
+ If you don’t want to share your IP address, do not click the button.
+ </p>
+ <button className="btn btn-primary" onClick={showNews}>Load News</button>
+ <label className="news-checkbox">
+ <input type="checkbox"
+ checked={isChecked}
+ onChange={toggleCookieSave}
+ />
+ Remember my choice
+ </label>
</div>
);
};
+class NewsPage extends React.Component {
+ constructor (props) {
+ super(props);
+ this.showNews = this.showNews.bind(this);
+ this.toggleCookieSave = this.toggleCookieSave.bind(this);
+
+ const hasCookie = !!app.utils.localStorageGet('allow-IP-sharing');
+
+ this.state = {
+ showNews: hasCookie ? true : false,
+ hasCookie
+ };
+ }
+
+ showNews() {
+ this.setState({ showNews: true });
+ }
+
+ toggleCookieSave() {
Review comment:
I think this function could just be:
```
app.utils.localStorageSet('allow-IP-sharing', !this.state.showNews);
this.setState({showNews: !this.state.showNews});
```
##########
File path: app/addons/news/components.js
##########
@@ -9,17 +9,81 @@
// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
// License for the specific language governing permissions and limitations under
// the License.
+import app from "../../app";
import React from "react";
-const NewsPage = () => {
+const LoadNewsButton = ({ showNews, isChecked, toggleCookieSave }) => {
Review comment:
Can you rename `toggleCookieSave` to `toggleChange`
##########
File path: app/addons/news/components.js
##########
@@ -9,17 +9,81 @@
// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
// License for the specific language governing permissions and limitations under
// the License.
+import app from "../../app";
import React from "react";
-const NewsPage = () => {
+const LoadNewsButton = ({ showNews, isChecked, toggleCookieSave }) => {
return (
- <div id="news-page" className="">
- <iframe src="https://blog.couchdb.org" width="100%" height="100%"></iframe>
+ <div>
+ <p>
+ When you click this button, you are requesting content and sharing your IP address with <a href="https://blog.couchdb.org/">blog.couchdb.org</a> which is edited by the Apache CouchDB PMC and maintained by <a href="https://wordpress.com/">wordpress.com</a>.
+ </p>
+ <p>
+ If you don’t want to share your IP address, do not click the button.
+ </p>
+ <button className="btn btn-primary" onClick={showNews}>Load News</button>
+ <label className="news-checkbox">
+ <input type="checkbox"
+ checked={isChecked}
+ onChange={toggleCookieSave}
+ />
+ Remember my choice
+ </label>
</div>
);
};
+class NewsPage extends React.Component {
+ constructor (props) {
+ super(props);
+ this.showNews = this.showNews.bind(this);
+ this.toggleCookieSave = this.toggleCookieSave.bind(this);
+
+ const hasCookie = !!app.utils.localStorageGet('allow-IP-sharing');
Review comment:
Could we remove the hasCookie and just have a `showNews`. I can't quite see why we would need both?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-fauxton] wohali commented on pull request #1292: Feat/news security info
Posted by GitBox <gi...@apache.org>.
wohali commented on pull request #1292:
URL: https://github.com/apache/couchdb-fauxton/pull/1292#issuecomment-689002519
Reminder to whomever merges this to please do the `grunt` and Fauxton release, including the push to npm. This is all that is holding up CouchDB 3.1.1.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-fauxton] garrensmith commented on a change in pull request #1292: Feat/news security info
Posted by GitBox <gi...@apache.org>.
garrensmith commented on a change in pull request #1292:
URL: https://github.com/apache/couchdb-fauxton/pull/1292#discussion_r484425171
##########
File path: app/addons/news/components.js
##########
@@ -9,17 +9,81 @@
// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
// License for the specific language governing permissions and limitations under
// the License.
+import app from "../../app";
import React from "react";
-const NewsPage = () => {
+const LoadNewsButton = ({ showNews, isChecked, toggleCookieSave }) => {
return (
- <div id="news-page" className="">
- <iframe src="https://blog.couchdb.org" width="100%" height="100%"></iframe>
+ <div>
+ <p>
+ When you click this button, you are requesting content and sharing your IP address with <a href="https://blog.couchdb.org/">blog.couchdb.org</a> which is edited by the Apache CouchDB PMC and maintained by <a href="https://wordpress.com/">wordpress.com</a>.
+ </p>
+ <p>
+ If you don’t want to share your IP address, do not click the button.
+ </p>
+ <button className="btn btn-primary" onClick={showNews}>Load News</button>
+ <label className="news-checkbox">
+ <input type="checkbox"
+ checked={isChecked}
+ onChange={toggleCookieSave}
+ />
+ Remember my choice
+ </label>
</div>
);
};
+class NewsPage extends React.Component {
+ constructor (props) {
+ super(props);
+ this.showNews = this.showNews.bind(this);
+ this.toggleCookieSave = this.toggleCookieSave.bind(this);
+
+ const hasCookie = !!app.utils.localStorageGet('allow-IP-sharing');
+
+ this.state = {
+ showNews: hasCookie ? true : false,
+ hasCookie
+ };
+ }
+
+ showNews() {
+ this.setState({ showNews: true });
+ }
+
+ toggleCookieSave() {
+ if (!this.state.hasCookie) {
+ this.setState(() => {
+ return { hasCookie: true };
+ });
+ app.utils.localStorageSet('allow-IP-sharing', true);
+
+ } else {
+ this.setState(() => {
+ return { hasCookie: false };
+ });
+ app.utils.localStorageSet('allow-IP-sharing', false);
+ }
+ }
+
+ render() {
+ return (
+ <div id="news-page" className="">
+ {this.state.showNews ?
Review comment:
Sorry, I'm not a big fan of that implementation. If you look in all the other Fauxton code we very really have the conditional in the jsx. I feel that is a lot more error-prone and much harder to read. If you don't link calling it news you can call it something else.
##########
File path: app/addons/news/components.js
##########
@@ -9,17 +9,81 @@
// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
// License for the specific language governing permissions and limitations under
// the License.
+import app from "../../app";
import React from "react";
-const NewsPage = () => {
+const LoadNewsButton = ({ showNews, isChecked, toggleCookieSave }) => {
return (
- <div id="news-page" className="">
- <iframe src="https://blog.couchdb.org" width="100%" height="100%"></iframe>
+ <div>
+ <p>
+ When you click this button, you are requesting content and sharing your IP address with <a href="https://blog.couchdb.org/">blog.couchdb.org</a> which is edited by the Apache CouchDB PMC and maintained by <a href="https://wordpress.com/">wordpress.com</a>.
+ </p>
+ <p>
+ If you don’t want to share your IP address, do not click the button.
+ </p>
+ <button className="btn btn-primary" onClick={showNews}>Load News</button>
+ <label className="news-checkbox">
+ <input type="checkbox"
+ checked={isChecked}
+ onChange={toggleCookieSave}
+ />
+ Remember my choice
+ </label>
</div>
);
};
+class NewsPage extends React.Component {
+ constructor (props) {
+ super(props);
+ this.showNews = this.showNews.bind(this);
+ this.toggleCookieSave = this.toggleCookieSave.bind(this);
+
+ const hasCookie = !!app.utils.localStorageGet('allow-IP-sharing');
Review comment:
Ok, fair. Since we not using Cookies could you rename it to something else.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-fauxton] hulkoba commented on a change in pull request #1292: Feat/news security info
Posted by GitBox <gi...@apache.org>.
hulkoba commented on a change in pull request #1292:
URL: https://github.com/apache/couchdb-fauxton/pull/1292#discussion_r482161860
##########
File path: app/addons/news/components.js
##########
@@ -9,17 +9,81 @@
// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
// License for the specific language governing permissions and limitations under
// the License.
+import app from "../../app";
import React from "react";
-const NewsPage = () => {
+const LoadNewsButton = ({ showNews, isChecked, toggleCookieSave }) => {
return (
- <div id="news-page" className="">
- <iframe src="https://blog.couchdb.org" width="100%" height="100%"></iframe>
+ <div>
+ <p>
+ When you click this button, you are requesting content and sharing your IP address with <a href="https://blog.couchdb.org/">blog.couchdb.org</a> which is edited by the Apache CouchDB PMC and maintained by <a href="https://wordpress.com/">wordpress.com</a>.
+ </p>
+ <p>
+ If you don’t want to share your IP address, do not click the button.
+ </p>
+ <button className="btn btn-primary" onClick={showNews}>Load News</button>
+ <label className="news-checkbox">
+ <input type="checkbox"
+ checked={isChecked}
+ onChange={toggleCookieSave}
+ />
+ Remember my choice
+ </label>
</div>
);
};
+class NewsPage extends React.Component {
+ constructor (props) {
+ super(props);
+ this.showNews = this.showNews.bind(this);
+ this.toggleCookieSave = this.toggleCookieSave.bind(this);
+
+ const hasCookie = !!app.utils.localStorageGet('allow-IP-sharing');
+
+ this.state = {
+ showNews: hasCookie ? true : false,
+ hasCookie
+ };
+ }
+
+ showNews() {
+ this.setState({ showNews: true });
+ }
+
+ toggleCookieSave() {
Review comment:
Yes :) the if/else was only there because of the cookie deletion. Forget to change
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-fauxton] hulkoba commented on a change in pull request #1292: Feat/news security info
Posted by GitBox <gi...@apache.org>.
hulkoba commented on a change in pull request #1292:
URL: https://github.com/apache/couchdb-fauxton/pull/1292#discussion_r482142156
##########
File path: app/addons/news/components.js
##########
@@ -9,17 +9,81 @@
// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
// License for the specific language governing permissions and limitations under
// the License.
+import app from "../../app";
import React from "react";
-const NewsPage = () => {
+const LoadNewsButton = ({ showNews, isChecked, toggleCookieSave }) => {
return (
- <div id="news-page" className="">
- <iframe src="https://blog.couchdb.org" width="100%" height="100%"></iframe>
+ <div>
+ <p>
+ When you click this button, you are requesting content and sharing your IP address with <a href="https://blog.couchdb.org/">blog.couchdb.org</a> which is edited by the Apache CouchDB PMC and maintained by <a href="https://wordpress.com/">wordpress.com</a>.
+ </p>
+ <p>
+ If you don’t want to share your IP address, do not click the button.
+ </p>
+ <button className="btn btn-primary" onClick={showNews}>Load News</button>
+ <label className="news-checkbox">
+ <input type="checkbox"
+ checked={isChecked}
+ onChange={toggleCookieSave}
+ />
+ Remember my choice
+ </label>
</div>
);
};
+class NewsPage extends React.Component {
+ constructor (props) {
+ super(props);
+ this.showNews = this.showNews.bind(this);
+ this.toggleCookieSave = this.toggleCookieSave.bind(this);
+
+ const hasCookie = !!app.utils.localStorageGet('allow-IP-sharing');
Review comment:
I think we need both, otherwise the news section is loaded by checking the checkbox (and I can never delete the value from the localstorage again) @garrensmith
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-fauxton] garrensmith merged pull request #1292: Feat/news security info
Posted by GitBox <gi...@apache.org>.
garrensmith merged pull request #1292:
URL: https://github.com/apache/couchdb-fauxton/pull/1292
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-fauxton] wohali commented on pull request #1292: Feat/news security info
Posted by GitBox <gi...@apache.org>.
wohali commented on pull request #1292:
URL: https://github.com/apache/couchdb-fauxton/pull/1292#issuecomment-685070211
Also please note https://github.com/apache/couchdb-fauxton/pull/1289 which seems to be a step we need before release, since this is holding up the 3.1.1 release.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-fauxton] garrensmith commented on pull request #1292: Feat/news security info
Posted by GitBox <gi...@apache.org>.
garrensmith commented on pull request #1292:
URL: https://github.com/apache/couchdb-fauxton/pull/1292#issuecomment-685014266
Also thanks for your first contribution 🎉 this is a good start. A few changes and this would be good to go.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-fauxton] hulkoba commented on pull request #1292: Feat/news security info
Posted by GitBox <gi...@apache.org>.
hulkoba commented on pull request #1292:
URL: https://github.com/apache/couchdb-fauxton/pull/1292#issuecomment-685621268
Thanks for feedback @garrensmith, I did all the changes :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-fauxton] garrensmith commented on a change in pull request #1292: Feat/news security info
Posted by GitBox <gi...@apache.org>.
garrensmith commented on a change in pull request #1292:
URL: https://github.com/apache/couchdb-fauxton/pull/1292#discussion_r481308073
##########
File path: app/addons/news/components.js
##########
@@ -11,14 +11,78 @@
// the License.
import React from "react";
+import Cookies from 'universal-cookie';
+const cookies = new Cookies();
-const NewsPage = () => {
+function LoadNewsButton(props) {
Review comment:
Can you make this a fat arrows function. `const LoadNewsButton => ({showNews, isChecked}`
##########
File path: app/addons/news/components.js
##########
@@ -11,14 +11,78 @@
// the License.
import React from "react";
+import Cookies from 'universal-cookie';
Review comment:
I'm not sure about adding a new dependancy to manage this? What about rather using localstorage to store this?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-fauxton] hulkoba commented on a change in pull request #1292: Feat/news security info
Posted by GitBox <gi...@apache.org>.
hulkoba commented on a change in pull request #1292:
URL: https://github.com/apache/couchdb-fauxton/pull/1292#discussion_r484981251
##########
File path: app/addons/news/components.js
##########
@@ -9,17 +9,73 @@
// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
// License for the specific language governing permissions and limitations under
// the License.
+import app from "../../app";
import React from "react";
-const NewsPage = () => {
+const LoadNewsButton = ({ showNews, isChecked, toggleChange }) => {
return (
- <div id="news-page" className="">
- <iframe src="https://blog.couchdb.org" width="100%" height="100%"></iframe>
+ <div>
+ <p>
+ When you click this button, you are requesting content and sharing your IP address with <a href="https://blog.couchdb.org/">blog.couchdb.org</a> which is edited by the Apache CouchDB PMC and maintained by <a href="https://wordpress.com/">wordpress.com</a>.
+ </p>
+ <p>
+ If you don’t want to share your IP address, do not click the button.
+ </p>
+ <button className="btn btn-primary" onClick={showNews}>Load News</button>
+ <label className="news-checkbox">
+ <input type="checkbox"
+ checked={isChecked}
+ onChange={toggleChange}
+ />
+ Remember my choice
+ </label>
</div>
);
};
+class NewsPage extends React.Component {
+ constructor (props) {
+ super(props);
+ this.showNews = this.showNews.bind(this);
+ this.toggleChange = this.toggleChange.bind(this);
+
+ const allowsIpSharing = !!app.utils.localStorageGet('allow-IP-sharing');
+
+ this.state = {
+ showNews: allowsIpSharing ? true : false,
+ allowsIpSharing
+ };
+ }
+
+ showNews() {
+ this.setState({ showNews: true });
+ }
+
+ toggleChange() {
+ const allowsIpSharing = this.state.allowsIpSharing;
+ this.setState(() => ({ allowsIpSharing: !allowsIpSharing }));
Review comment:
@garrensmith I did it :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-fauxton] dottorblaster commented on a change in pull request #1292: Feat/news security info
Posted by GitBox <gi...@apache.org>.
dottorblaster commented on a change in pull request #1292:
URL: https://github.com/apache/couchdb-fauxton/pull/1292#discussion_r482145544
##########
File path: app/addons/news/components.js
##########
@@ -9,17 +9,81 @@
// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
// License for the specific language governing permissions and limitations under
// the License.
+import app from "../../app";
import React from "react";
-const NewsPage = () => {
+const LoadNewsButton = ({ showNews, isChecked, toggleCookieSave }) => {
return (
- <div id="news-page" className="">
- <iframe src="https://blog.couchdb.org" width="100%" height="100%"></iframe>
+ <div>
+ <p>
+ When you click this button, you are requesting content and sharing your IP address with <a href="https://blog.couchdb.org/">blog.couchdb.org</a> which is edited by the Apache CouchDB PMC and maintained by <a href="https://wordpress.com/">wordpress.com</a>.
+ </p>
+ <p>
+ If you don’t want to share your IP address, do not click the button.
+ </p>
+ <button className="btn btn-primary" onClick={showNews}>Load News</button>
+ <label className="news-checkbox">
+ <input type="checkbox"
+ checked={isChecked}
+ onChange={toggleCookieSave}
+ />
+ Remember my choice
+ </label>
</div>
);
};
+class NewsPage extends React.Component {
+ constructor (props) {
+ super(props);
+ this.showNews = this.showNews.bind(this);
+ this.toggleCookieSave = this.toggleCookieSave.bind(this);
+
+ const hasCookie = !!app.utils.localStorageGet('allow-IP-sharing');
+
+ this.state = {
+ showNews: hasCookie ? true : false,
+ hasCookie
+ };
+ }
+
+ showNews() {
+ this.setState({ showNews: true });
+ }
+
+ toggleCookieSave() {
+ if (!this.state.hasCookie) {
+ this.setState(() => {
+ return { hasCookie: true };
+ });
+ app.utils.localStorageSet('allow-IP-sharing', true);
+
+ } else {
+ this.setState(() => {
+ return { hasCookie: false };
+ });
+ app.utils.localStorageSet('allow-IP-sharing', false);
+ }
+ }
+
+ render() {
+ return (
+ <div id="news-page" className="">
+ {this.state.showNews ?
Review comment:
I don't feel very comfortable with `let`ting and reassigning, are you sure about that?
(sorry for the unsoliticited feedback)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-fauxton] janl commented on a change in pull request #1292: Feat/news security info
Posted by GitBox <gi...@apache.org>.
janl commented on a change in pull request #1292:
URL: https://github.com/apache/couchdb-fauxton/pull/1292#discussion_r481892600
##########
File path: app/addons/news/components.js
##########
@@ -11,14 +11,78 @@
// the License.
import React from "react";
+import Cookies from 'universal-cookie';
Review comment:
that would also work, I suggested cookies because of how this is usually done in authentication / remember me type situations, but localStorage is fine, too
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-fauxton] wohali commented on pull request #1292: Feat/news security info
Posted by GitBox <gi...@apache.org>.
wohali commented on pull request #1292:
URL: https://github.com/apache/couchdb-fauxton/pull/1292#issuecomment-685001766
@hulkoba Thank you for your first-time contribution to Apache CouchDB Fauxton! 👯 ⛹️♀️ 🚴
I'm hoping that @Antonio-Maranhao or @popojargo can review this. Either of you, if this lands shortly, please cut a new Fauxton release so I can put this into 3.1.1.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-fauxton] wohali commented on pull request #1292: Feat/news security info
Posted by GitBox <gi...@apache.org>.
wohali commented on pull request #1292:
URL: https://github.com/apache/couchdb-fauxton/pull/1292#issuecomment-689015846
Thanks @garrensmith - can you or @Antonio-Maranhao or @popojargo do the merge and release dance? Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-fauxton] dottorblaster commented on a change in pull request #1292: Feat/news security info
Posted by GitBox <gi...@apache.org>.
dottorblaster commented on a change in pull request #1292:
URL: https://github.com/apache/couchdb-fauxton/pull/1292#discussion_r484376139
##########
File path: app/addons/news/components.js
##########
@@ -9,17 +9,81 @@
// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
// License for the specific language governing permissions and limitations under
// the License.
+import app from "../../app";
import React from "react";
-const NewsPage = () => {
+const LoadNewsButton = ({ showNews, isChecked, toggleCookieSave }) => {
return (
- <div id="news-page" className="">
- <iframe src="https://blog.couchdb.org" width="100%" height="100%"></iframe>
+ <div>
+ <p>
+ When you click this button, you are requesting content and sharing your IP address with <a href="https://blog.couchdb.org/">blog.couchdb.org</a> which is edited by the Apache CouchDB PMC and maintained by <a href="https://wordpress.com/">wordpress.com</a>.
+ </p>
+ <p>
+ If you don’t want to share your IP address, do not click the button.
+ </p>
+ <button className="btn btn-primary" onClick={showNews}>Load News</button>
+ <label className="news-checkbox">
+ <input type="checkbox"
+ checked={isChecked}
+ onChange={toggleCookieSave}
+ />
+ Remember my choice
+ </label>
</div>
);
};
+class NewsPage extends React.Component {
+ constructor (props) {
+ super(props);
+ this.showNews = this.showNews.bind(this);
+ this.toggleCookieSave = this.toggleCookieSave.bind(this);
+
+ const hasCookie = !!app.utils.localStorageGet('allow-IP-sharing');
+
+ this.state = {
+ showNews: hasCookie ? true : false,
+ hasCookie
+ };
+ }
+
+ showNews() {
+ this.setState({ showNews: true });
+ }
+
+ toggleCookieSave() {
+ if (!this.state.hasCookie) {
+ this.setState(() => {
+ return { hasCookie: true };
+ });
+ app.utils.localStorageSet('allow-IP-sharing', true);
+
+ } else {
+ this.setState(() => {
+ return { hasCookie: false };
+ });
+ app.utils.localStorageSet('allow-IP-sharing', false);
+ }
+ }
+
+ render() {
+ return (
+ <div id="news-page" className="">
+ {this.state.showNews ?
Review comment:
@hulkoba much better now, thanks for taking the comment under consideration :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org