Skip to content
This repository was archived by the owner on May 12, 2021. It is now read-only.

Conversation

@Happy0
Copy link
Contributor

@Happy0 Happy0 commented Feb 13, 2018

A user's activity (posts, likes, etc) will not be visible to anyone under the following circumstances:

  1. They onboard via a local network by following someone on the network. However, nobody on that network follows them back before they leave the network and open patchwork back up on another network. They can therefore see content, but nobody can see theirs.

  2. They redeem a pub invite, but due to a bug (or the server spontaneously exploding at just the wrong moment, and the key is lost in the wreckage) the pub never follows the user back. This has happened to me and I have seen it happen to at least one friend (a pub invite following the pub, resulting in gossip to download new messages, but the pub not following back.) Luckily, I was able to notice this had happened to my friend and tell them to try another pub.

I plan to investigate the scuttlebot side of this to see if I can work out why this sometimes happens, but I still think it's worth being defensive 'client side' as there may be a regression, a faulty implementation, a spontaneous server explosion, etc.

I think it's worth having a warning message for these situations.

Note: the warning box is only displayed if the user follows someone or a pub, but is not followed back See the code comments for an explanation of why I took this approach.

Does this approach to issues 1 and 2 seem reasonable?

This pull request follows on from #713

Copy link
Contributor

@mmckegg mmckegg left a comment

Choose a reason for hiding this comment

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

Looking pretty good. Haven't yet had a chance to try this out though.

Oh, I suggest you run npm test (which runs standard JS) and fix up the formatting issues (there's just a couple)

locales/en.json Outdated
"unblocked ": "unblocked ",
"identified ": "identified "
"identified ": "identified ",
"Nobody will be able to see your posts until you have a follower. The easiest way to get a follower is to use a pub invite as the pub will follow you back. If you have already redeemed a pub invite and it has not followed you back, try another pub.": "Nobody will be able to see your posts until you have a follower. The easiest way to get a follower is to use a pub invite as the pub will follow you back. If you have already redeemed a pub invite and it has not followed you back, try another pub.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like there is an unused locale string here.

"Nobody will be able to see your posts until you have a follower. The easiest way to get a follower is to use a pub invite as the pub will follow you back. If you have already redeemed a pub invite and it has not followed you back, try another pub."

Copy link
Contributor

Choose a reason for hiding this comment

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

oh, you fixed this just before I commented 👍

@Happy0
Copy link
Contributor Author

Happy0 commented Feb 14, 2018

Thanks @mmckegg =]. I ran 'npm test'. I like that there's a flag to make it try and fix some of the issues for you automatically :D (which I did.)

I left the unused variables reported by standard in files I didn't touch as I assume they're there for convenience.

@Happy0
Copy link
Contributor Author

Happy0 commented Feb 14, 2018

Oh, I forgot to fix the formatting of

`
var {

  • when,
  • h
    +} = require('mutant')
    `

I'll do that later. Goodnight! :)

@Happy0
Copy link
Contributor Author

Happy0 commented Feb 14, 2018

done =]

@Happy0
Copy link
Contributor Author

Happy0 commented Feb 18, 2018

Updated the pull request to check 'sync' (indexing has finished) like the recent change for the 'no followers' warning.

'Nobody will be able to see your posts until you have a follower. The easiest way to get a follower is to use a pub invite as the pub will follow you back. If you have already redeemed a pub invite and you see it has not followed you back on your profile, try another pub.'
)

// We only show this if the user has followed someone as the first warning ('you are not followed anyone')
Copy link
Contributor

Choose a reason for hiding this comment

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

followed => following

}

function noFollowersWarning () {
var explanation = i18n(
Copy link
Contributor

Choose a reason for hiding this comment

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

I hope at some point we can start transitioning to const over var. Though not necessarily in this change.

"minutes": "mins",
"seconds": "secs",
"minutes": "minutes",
"seconds": "seconds",
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these lines related to the purpose of this change?

* An error in the code comments
* Use 'const' over let in some functions I introduced.
@Happy0
Copy link
Contributor Author

Happy0 commented Feb 19, 2018

Cheers @hbetts =].

I pushed a new commit. I fixed the code comment you pointed out (well spotted.) I changed var to 'const' in my local functions in modules/page/html/render/public.js . There is a mixture of const and var in that file, so I thought it was fine to make this change in the functions I added. However, I'd rather leave changing the rests of the vars to const / let for a separate pull request unless @mmckegg wants me to change the rest of the file too.

locales/en.json is auto-generated, which is why you'll often see changes in it that seem unrelated with commits and pull requests @hbetts .

@hutson
Copy link
Contributor

hutson commented Feb 19, 2018

locales/en.json is auto-generated, which is why you'll often see changes in it that seem unrelated with commits and pull requests

Ah. Thank you for the explanation. 😄

However, I'd rather leave changing the rests of the vars to const / let for a separate pull request

I definitely agree with waiting until a later pull request. It's not a change directly related to the goal of this pull request.

From what else I can tell, this change looks good to me 👍

@Happy0
Copy link
Contributor Author

Happy0 commented Feb 19, 2018

\o/ :D

"one": "You follow someone that follows this person.",
"other": "You follow %s people that follow this person."
"one": "You follow %s people that follow this person.",
"other": "You follow %s people that follow this person."
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure why these changes were made. It is overriding the custom strings with default ones. I will attempt to roll these back before merging. Hopefully my git foo is strong enough!!

@mmckegg mmckegg merged commit 326fb93 into ssbc:master Feb 20, 2018
mmckegg added a commit that referenced this pull request Feb 20, 2018
@mmckegg
Copy link
Contributor

mmckegg commented Feb 20, 2018

Released as part of Patchwork v3.8.9 🎉

@Happy0
Copy link
Contributor Author

Happy0 commented Feb 20, 2018

Yasss :D.

mmckegg added a commit that referenced this pull request Feb 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants