Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Layout #1

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

Layout #1

wants to merge 14 commits into from

Conversation

Cado21
Copy link

@Cado21 Cado21 commented Sep 9, 2019

.env template ada di folder server "env-template"

CLIENT_ID=397711291646-fgrnl8h2969mqm5fd2ufmjthm4kv36p5.apps.googleusercontent.com
PORT=3000
DATABASE_URL=mongodb://localhost:27017/mini-wp
SECRET_JWT=SECRETWOI
SALT_ROUND=10
DEFAULT_PASSWORD=DEFAULT
CLOUD_BUCKET=deeppress
GCLOUD_PROJECT=deeppress
KEYFILE_PATH=keyfile.json

Feature Tambahan:

  1. Article bisa diberi Tag
  2. Filter bukan cuma berdasarkan title tetapi tag juga ada
  3. Ada page untuk melihat detail artikel yang di post

Kendala :

  1. Website deploy lemot, ketika barusan login trus ganti orang lain login lama fetchnya..
  2. suka kena status bad gateway

link : http://deeppress.cado.store/

@@ -0,0 +1,9 @@
CLIENT_ID=397711291646-fgrnl8h2969mqm5fd2ufmjthm4kv36p5.apps.googleusercontent.com
Copy link
Collaborator

Choose a reason for hiding this comment

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

ketika membuat template, tidak perlu memasukan credential valuenya

server/app.js Outdated
mongoose.connect( DATABASE_URL , { useNewUrlParser : true })
.then( () => { console.log( `Database connected to: ${DATABASE_URL}` ); })
.catch( err =>{ console.log( err ); })
mongoose.set('useCreateIndex', true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

bisa dijadikan satu di confignya saat connect

type : String,
required : true
},
email : {
Copy link
Collaborator

Choose a reason for hiding this comment

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

kenapa tidak dibuat untuk check unique?

let status ;
let message;

if ( err.name == 'ValidationError' ) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

jangan hanya seperti ini, jadi sia-sia jika sudah membuat custom message di validasi model. gunakan custom message untuk dikirimkan ke client


// Check if the user bring token; if not let user know they must login
function authentication ( req ,res , next ) {
try {
Copy link
Collaborator

Choose a reason for hiding this comment

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

tips: ada pengecekan user apakah masih ada atau tidak di database

@@ -0,0 +1,117 @@
const Article = require('../models/article');
class ArticleController {
Copy link
Collaborator

Choose a reason for hiding this comment

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

tidak selalu semuanya populate data user, karena akan mendapatkan passwordnya

})
return;
}
if ( !this.file ) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

jika ketika edit tidak mengirimkan photo, bisa menggunakan photo yang lain

@WikaSilo
Copy link
Collaborator

Cado, sekali lagi mengingatkan yaa :D
untuk styling penulisan mu yang seperti ini:

app.use(logger('dev'))
app.use( cors() )

mohon konsistensi ketika memberika spasi setelah (), jika memang itu yang dipilih berarti semuanya juga harus diubah seperti itu, semisal:

app.use( express.urlencoded({ extended : true }));
// seharusnya menjadi
app.use( express.urlencoded({ extended : true }) );

jadi kalo diperhatiin ada ketidak konsistenan (biasanya) di akhir coding

berikut tambahan lainnya:

  • ketika update article, UX sebenarnya tidak perlu upload gambar lain jika memang gambarnya tidak ingin diubah, bisa di handle di state

overall sudah OK, jika ada tambahan dan perubahan, kabari instructor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants