README is misleading about details of example user

[jareddonovan]

Nature of issue?

• Found a bug
• Existing feature enhancement
• New feature request

Details about the bug:

• Web browser and version: Chrome 65.0.3325.181
• Operating System: MacOS 10.13.4
• Steps to reproduce this:
• The README.md file says that when the npm run fetch-examples command is run, a user called "p5" will be created. But actually, the user created depends on the values that have been entered into the .env file.
• To verify this, give a different value for EXAMPLE_USERNAME in .env
• Run the npm run fetch-examples command.
• Run mongo (with mongodb running).
• In the mongo console, run use p5js-web-editor, then db.users.find({}). You can see that the created user has the username you specified.

Feature enhancement details:

This seems like it would just need a simple rewording of the relevant section of the README. I will create a pull request that includes this and add a reference to this issue.

Sorry if this seems nit-picky. I'm just trying to get my environment set up (and understand what's happening around that) and I figure the smoother you can make this for new contributors, the better.

[jareddonovan]

After some further testing, I realised that there is a related issue - that the fetch-examples script seems to expect the user to have username "p5", even if something different is specified in the .env file.

When I create a user with a different username (e.g. "example") the fetch-examples script creates that user, but the examples don't seem to be added to it. When I log in as "example", I can log in fine, but the examples don't show up. If I change the example user details in README to be "p5", and then run the fetch-examples script, the examples show up fine.

The following image is what I see for an example user called "example" when I chose File | Examples

The following is what I get when I do the same thing for an example user called "p5".

[shinytang6]

Hi @awarua! You can see what happened there in #556.

I don't know why the error occurs, maybe there are some previous methods in server/examples.js have not been corrected.

[jareddonovan]

I am still trying to understand what is happening here. From what I have been able to figure out by looking through the fetch-examples.js script, the example projects are being added to the example user specified in .env, but for some reason, they are not showing up when you click to view the examples in the editor.

Could it be that the p5 username is hardcoded in other parts of the app rather than being read from the environment? I'm still a total noob with react, but it looks to me like the username of the example user is assumed in the following code in Nav.jsx (Line 219) and this is the reason that the examples don't show up when you have an example user called something other than "p5".

                <Link
                  to="/p5/sketches"
                  onFocus={this.handleFocus.bind(this, 'file')}
                  onBlur={this.handleBlur}
                  onClick={this.setDropdown.bind(this, 'none')}
                >
                  Examples
                </Link>

It looks like there's a similar problem on the 404Page.js on Line 5. I also get crashes when I come across a 404 page with an example user called something different.

User.findOne({ username: 'p5' }, (userErr, user) => { // Find p5 user
  if (userErr) {
    throw userErr;
}

It's also kind of assumed in the log messages for the fetch-examples and fetch-examples-gg scripts (Lines 281 and 452 respectively):

console.log(`Created a new project in p5 user: ${savedProject.name}`);

I had originally thought that this issue would be resolved with a simple rewording of the README file, but I don't think that's adequate. If a developer puts a name other than 'p5' into the .env file, they are going to run into problems. It seems to me that either:

1. EXAMPLE_USERNAME should not be a settable option or...
2. The code above should read values from the environment rather than assuming that the example user will be called "p5"

I'll leave this here and await your guidance on what is the best way to address this.

[shinytang6]

I just provide some of my views for reference.
What's happening now is that if you log in with e.g. the example user, click on Nav->example and it will show p5 user's sketches, but p5 user may not exist. However, if you click on My account->sketches or Nav-> Open, it will still show the sample sketches downloaded through npm run fetch-examples, correct me if l am wrong :)

I think the current practice is to change the Nav route to something like {/${this.props.user.username}/sketches}. But I think it's better to separate the example sketches from the sketches we wrote in the future because it looks like there is no difference between Nav->example and My account->sketches if we do so ..

As for the log messages in fetch-examples.js and fetch-examples-gg.js, l think we can replace p5 with process.env.EXAMPLE_USERNAME

[jareddonovan]

Thanks for the comment @shinytang6. It's helpful for me to get your explanation of what is happening.

Correct me if I'm wrong, but my assumption here is that the user that is defined in the .env file seems mainly intended as a 'placeholder' user for attaching examples to (regardless of whether we call it "example" or "p5"). At least, the only place those config variables are used in the code is to create the example user. Perhaps it is also intended as a convenience for local development too?

Regarding the behavior you describe, – yes, that is what I get. When I set the example user in .env to have the username "example", I can log in with that username. If I click on 'File->Examples', I see nothing (as screenshotted above), but if I click on File->Open or My Account->Sketches, I can see the sketches as shown below:

So the user is definitely defined and the example sketches are correctly associated with it in the database. The problem is that the assumption that the example user will be called p5 leads to problems elsewhere in the code. If I try to navigate to a non-existent page, http://localhost:8000/not-there, the server crashes with the following error.

[CUT]/p5.js-web-editor/server/views/404Page.js:24
      _project2.default.find({ user: user._id }, function (projErr, projects) {
                                          ^

TypeError: Cannot read property '_id' of null

I have created a pull request that uses the user-defined environment variable rather than hard-coded values in the locations I mentioned above (PR #648). This pull request lets you define an example user with whatever details you like and it these are used instead of "p5". If you define "example" as the username rather than p5, examples will be located at /example/sketches.

Even if I have totally misunderstood the meaning of these variables and the above is not a good solution, this has been a good learning experience for me to start to see how the project fits together. Thanks very much for your help 👍.

[shinytang6]

Never mind, l am also learning 😄

Correct me if I'm wrong, but my assumption here is that the user that is defined in the .env file seems mainly intended as a 'placeholder' user for attaching examples to (regardless of whether we call it "example" or "p5"). At least, the only place those config variables are used in the code is to create the example user. Perhaps it is also intended as a convenience for local development too?

That's true, we need a 'placeholder' user to fetch the examples. That's why we use EXAMPLE_USERNAME here. It should have been designed as an unchangeable user(l mean keeping the examples pure). But generally, we develop locally with this username.

[catarak]

i appreciate both of you digging into this! i feel like this issue needs a little context.

i originally made the decision to put the examples under a username called p5 because i wanted to figure out the quickest way to add the example sketches to the web editor with the least number of changes. i figured in the future there would eventually be a way to add sketches that don't have an owner, or maybe we'd put these under an /examples route and obscure the username in this special case or something like that.

at some point, i realized that the p5 user's password was stored in the fetch-examples script, so i thought it made sense to move it to environment variables. i didn't realize that it would be confusing if you put a different username for the EXAMPLE_USERNAME.

for now, in production, i don't think the username should be anything different from p5. i think it would be confusing to users and make it unclear as to whether these were 'official' p5 examples. so maybe it would make sense to move this out of an environment variable rather than allow for it to be anything.

as a sidenote, the reason that the script fetch-examples-gg got added was because there's a new version of the book Generative Design (Generative Gestaltung in German) which translated the examples from Processing to p5, and @joeyklee approached me and asked if there was a way to bulk upload sketches to the web editor. Because of the imminent release and need to print hyperlinks to example sketches, we worked together to make a working solution while #542 is not implemented yet.

hopefully this is helpful! it's awesome to have folks looking at this repository and finding issues.

[jareddonovan]

Hi @catarak, sorry for the delay in responding. I only get a chance to contribute on weekends :).

That background makes perfect sense. I have made a different pull request that removes EXAMPLE_USERNAME as a settable option from .env, I agree that this would remove the confusion (#651).

Regarding the fetch-examples-gg script. From what I can see of that it uses a different example user called generative-design with associated environment variables GG_EXAMPLES_USERNAME, GG_EXAMPLES_PASS, and GG_EXAMPLES_EMAIL. From what you describe above @catarak, this seems like it was a quick fix to the bulk upload problem #542. Since these env variables are not in the README.md or .env.example files, I don't think they're going to cause confusion and I have left them as they are.

[catarak]

@awarua thanks so much for your work on this! I just merged your PR, and yes, I think it makes sense to leave the Generative Design stuff in place until you can upload sketches to the editor via an API.