Insert link to email into "contact us" phrases

[nathalier]

Proposed changes in this pull request

• Insert link to email into "contact us" phrases.
  See "Contact us" should have emailto link #631

When should this PR be merged

Anytime

Risks

None

Follow up actions

Proceed with translation in Transifex

[ian-ross]

Hi @nathalier! Thanks for the PR. There are two things you need to do for us to be able to merge this (you will want to read to the bottom, even though this is quite long):

1. You've been committing work directly to the master branch of your repo, and you made the PR from master. That's not a good thing to do, because it can make it difficult to incorporate changes from the "upstream" Cadasta/cadasta-platform repository into your work, which you need to do to keep your PR branch up to date with other work that's going on in the main repo. The way to deal with this is to work on what people call a topic (or feature) branch. When you want to work on a feature, you check out your master branch, then do something like git checkout -b bugfix/#675, which makes a new branch with a mnemonic name, expecially for the work you're going to do, so that we can keep track of what's going on.
2. You'll see that GitHub says "This branch is out-of-date with the base branch". That means that there have been changes in the main Cadasta/cadasta-platform repository that you don't have in your PR branch. That means that there's no way to know if your work is compatible with what's already been done. What you need to do in this case is to make sure that you've added our repo as an upstream remote for your fork (GitHub usually tells you to do this when you create a fork), fetch the latest master from there, and then update your PR branch to incorporate the latest changes. (This is the point where working directly on master in your repo turns out to be a bad idea...) If you're working on a topic branch, you do this:

git checkout master  # This is your local master branch
git fetch upstream/master  # Get changes from Cadasta/cadasta-platform
git merge upstream/master  # This does a "fast-forward" merge
git checkout <topic-branch>  # Switch back to your topic branch
git rebase master  # Replay the changes you've made on top of the latest upstream master
git push --force  # Update your PR on GitHub

If there are conflicts between your work and what's been done in the main repo, after you do the rebase, you'll get merge conflicts that you need to fix.

This workflow only works if you use topic branches. If you work directly on your local master branch, then the merge of upstream/master is not a fast-forward merge (because you have changes to your local master) and things quickly get ugly.

What I suggest: Start over. Save the changes you've made somewhere so you don't forget what you did. Then delete your cadasta-platform repo and make a new fork from the Cadasta/cadasta-platform repo (this is just to make sure that the master branch in your fork is in a good state). Then, make a topic branch, apply the necessary changes (that you saved...), and push the topic branch to your fork of the repo on GitHub. Then you can make a new PR from that topic branch, and everything will be much easier to deal with.

[nathalier]

Sorry for not following the workflow with precision, and thanks for spoon-feeding :) Please see new version of this pull request: #684

[ian-ross]

@nathalier Not really spoon-feeding. Dealing with Git is something that a lot of even quite experienced programmers have trouble with. I still get into a tangle sometimes. It's useful to gain some familiarity with good working practices on simple issues like this one where the code changes are small, so you can concentrate on what's going on with Git. Anyway, good work on the PR! Why don't you see if you can find a slightly more difficult issue to fix now?

[nathalier]

@ian-ross Thank you! I was looking at this one: #609 , but I haven't yet comprehended completely. It looks like in general the approach should be to define view_allowed in .... presumably OrganizationMembers class and then use it in template. But I'm not sure it's correct.

[ian-ross]

I think the first step there is to define exactly what behaviour is expected, then to write some tests that discriminate between correct and incorrect behaviour. By the time you've done that, you'll probably know pretty clearly how to fix it. If you do want to start on that issue, I'd suggest starting off by writing a clear comment on the issue describing exactly what behaviour should be seen under what circumstances. People can then argue about it a bit and you can refine that description until it's completely clear. Then you can write some tests.