Skip to content
This repository has been archived by the owner on Feb 13, 2021. It is now read-only.

Create input form #26

Merged
merged 60 commits into from
Nov 26, 2020
Merged

Create input form #26

merged 60 commits into from
Nov 26, 2020

Conversation

yusuke-sforzando
Copy link
Contributor

@yusuke-sforzando yusuke-sforzando commented Nov 20, 2020

Describe the PR

To close #9 .
Request input text value to GET /search.

Screenshots

index.html with no error

Screen Shot 2020-11-26 at 14 52 38

search.html on success

Screen Shot 2020-11-26 at 14 52 49

Detail of the change

  1. index.html
    • Show title "Registration of equipment and books"
    • Show label "Enter one of the following keywords: keyword, ISBN code, or ASIN code"
    • Putting up a text input form
    • Putting up a submit button showed「Search」
    • GET the string in the input form to /search when the submit button is pressed
  2. search.html
    • The string to be GETed is displayed as is

Anticipated impacts

None.

To reproduce

  1. Run python main.py
  2. Visit to http://0.0.0.0:8888/
  3. Make sure the screen is displayed as shown in the screenshot
  4. Type in the appropriate string and press the search button
  5. Make sure you see the string you entered

Additional context

CSS implemented in Issue #18

Next issue is #11

@yusuke-sforzando yusuke-sforzando added the enhancement New feature or request label Nov 20, 2020
@yusuke-sforzando yusuke-sforzando added this to the Version 0.9 milestone Nov 20, 2020
@yusuke-sforzando yusuke-sforzando self-assigned this Nov 20, 2020
@codecov
Copy link

codecov bot commented Nov 20, 2020

Codecov Report

Merging #26 (fd33994) into main (0dbd2e4) will increase coverage by 1.88%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #26      +/-   ##
==========================================
+ Coverage   79.01%   80.89%   +1.88%     
==========================================
  Files           4        4              
  Lines          81       89       +8     
  Branches        6        7       +1     
==========================================
+ Hits           64       72       +8     
  Misses         12       12              
  Partials        5        5              
Impacted Files Coverage Δ
main.py 88.23% <100.00%> (+10.45%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0dbd2e4...fd33994. Read the comment docs.

main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
templates/index.html Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
templates/index.html Outdated Show resolved Hide resolved
templates/index.html Outdated Show resolved Hide resolved
templates/search-result.html Outdated Show resolved Hide resolved
@yusuke-sforzando yusuke-sforzando marked this pull request as ready for review November 21, 2020 07:26
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated
app.logger.info("search_result(): GET /")
keyword = request.args.get("input_keyword", "")
if keyword:
return render_template("search-result.html", keyword=keyword)
Copy link
Collaborator

Choose a reason for hiding this comment

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

元あった context_dict を活かして、keyword を追加する方が良いかな。
なぜなら、この後context_dictには、Amazon PA-APIで取得した結果とか、default_positionsとかそう言うのが追加されていくことが想定できるから。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

確かにそうですね。
修正します。

main.py Outdated Show resolved Hide resolved
templates/index.html Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
main.py Outdated Show resolved Hide resolved
templates/index.html Outdated Show resolved Hide resolved
@yusuke-sforzando
Copy link
Contributor Author

失礼しました。
修正します。

{% extends "_base.html" %}
{% block main %}
<h1>Search Result</h1>
<h2>Keyword: {{ keyword }}</h2>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Flash messageを実装したらそもそもTopページにリダイレクトさせてからのメッセージ表示とかにできたらと思っているのだけど、

現時点では、messageが表示される = Keyword がない というない状況で、keyword: と表示されているのは変だから、

{% if not message %}
<h2>Keyword: {{ keyword }}</h2>
{% endif %}

の場合分けをするのはどうでしょうか?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

そうですね。その方が良いですね。
修正します。

Copy link
Collaborator

Choose a reason for hiding this comment

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

いや、むしろ index.html に戻してあげなよ。message付きで。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

なるほどですね。
そうですね、そちらの方がユーザーファーストなのでそちらでやってみます。

Copy link
Collaborator

Choose a reason for hiding this comment

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

index.html がレンダーされた、と言うテストが必要なのでは?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

こちらもそうですね。
追記します。

main.py Outdated
"message": "This is dummy message."
"subtitle": f"Search Result for {keyword}",
"keyword": keyword,
"message": None if keyword else "Enter keywords back on the top page."
Copy link
Collaborator

Choose a reason for hiding this comment

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

index.htmlで表示されるメッセージなら、文章としては、"Enter any keywords." で良さそうですね

Copy link
Contributor Author

Choose a reason for hiding this comment

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

そうですね。
修正します。

main.py Outdated

from __init__ import app

app.secret_key = "aasss"
Copy link
Collaborator

Choose a reason for hiding this comment

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

このPRでは、不要と思います。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

そうですね、修正します。

if keyword:
return render_template("search.html", **context_dict)
else:
return render_template("index.html", **context_dict)
Copy link
Collaborator

Choose a reason for hiding this comment

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

これでは index.htmltitleSearch Result for になります。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

そうですね。
context_dictsubtitleを変更するコードを追記します。

@@ -1,3 +1,9 @@
{% extends "_base.html" %}
{% block main %}
<h1>Registration of equipment and books</h1>
<form action="/search" method="get">
<label for="query">Enter one of the following keywords: keyword, ISBN code, or ASIN code </label>
Copy link
Collaborator

Choose a reason for hiding this comment

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

英文なんだから末尾にピリオドが要りますよね。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

失礼しました。追記します。

{% extends "_base.html" %}
{% block main %}
<h1>Search Result</h1>
<h2>Keyword: {{ keyword }}</h2>
Copy link
Collaborator

Choose a reason for hiding this comment

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

index.html がレンダーされた、と言うテストが必要なのでは?

main.py Outdated
if keyword:
return render_template("search.html", **context_dict)
else:
context_dict["subtitle"] = "a service to quickly register equipments and books."
Copy link
Collaborator

Choose a reason for hiding this comment

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

<title>q-lako - {{ subtitle | default("a service to quickly register equipments and books") }}</title>
にて default が設定されてるので要らないです。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

承知しました!修正します!

main.py Outdated
@app.route("/search", methods=["GET"])
def search():
app.logger.info(f"search(): GET {request.full_path}.")
keyword = request.args.get("query", None)
context_dict = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

keyword の有無で処理が変わるので、
それぞれのif文の中でそれぞれに必要最低限の context_dict を作った方が良さそう。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

なるほどですね。
承知しました、修正します!

Copy link
Collaborator

@tomoya-sforzando tomoya-sforzando left a comment

Choose a reason for hiding this comment

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

確認しました

@tomoya-sforzando tomoya-sforzando merged commit 5ea5ed6 into main Nov 26, 2020
@tomoya-sforzando tomoya-sforzando deleted the 009_Create_input_form branch November 26, 2020 08:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create an input form
3 participants